Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] - Inform when there is no minor version #668

Draft
wants to merge 3 commits into
base: 8.0.x
Choose a base branch
from

Conversation

Ferror
Copy link

@Ferror Ferror commented Oct 21, 2022

I had probably the edge-case when my library had no minor versions. Only preleases. I was getting information that there are no versions at all. Which is false. There are versions. I would like just to improve the DX and show the proper message :)

In CollectionIsEmpty.php line 13:
                                                         
  Cannot get the first Version from an empty collection  
                                                         

@@ -24,6 +24,10 @@ public function assert(Version $version): bool
return ! $version->isPreRelease();
}
});

if ($stableVersions->isEmpty()) {
throw new \Exception('Your library does not have any minor version');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is a crash really the best approach? Perhaps it should be "green" when no BC breaks are detected instead?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback. That's why I kinda put it as a draft with WIP. And yes when there is no stable version - there won't be any BC breaks. Note or Info message could be applied.

Stay tuned 😄

@Ocramius Ocramius added this to the 8.0.0 milestone Nov 13, 2022
@Ocramius Ocramius changed the base branch from 7.4.x to 8.0.x November 13, 2022 13:05
Comment on lines +127 to +130
if ($stableVersions->isEmpty()) {
$stdErr->writeln(Str\format('Your library does not have any stable versions. Therefore cannot have BC breaks.'));

return Command::SUCCESS;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just realized another problem, sorry @_@

Assume somebody incorrectly installs this tool with a CI setup that did not pull tags during clone operations (shallow clone: default on github), this leads to an exit code zero by default, hiding the underlying problem.

Perhaps an exception or non-zero exit code was really the best approach.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, maybe we can kill two birds with one stone. One is a misconfiguration which we could verify and the second is an exception :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One is a misconfiguration which we could verify

How do we verify it, if no git fetch has occurred? 🤔

Copy link
Author

@Ferror Ferror Nov 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't that...

No tags -> possible misconfiguration so we can throw an exception. Like. THERE ARE NO TAGS. WE NEED THEM (yes, we gonna scream 😄)

Then at some point, we can assume that there are at least some tags (we verified that by the previous action), but we want to check out the stable ones. There are tags, but none of them are stable. Return success.

What do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

THERE ARE NO TAGS. WE NEED THEM (yes, we gonna scream smile)

Works for me - indeed the tool cannot do auto---from= without tags.

Then at some point, we can assume that there are at least some tags (we verified that by the previous action), but we want to check out the stable ones. There are tags, but none of them are stable. Return success.

Good enough :-)

@Ocramius Ocramius removed this from the 8.0.0 milestone Nov 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants