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

"prerelease" keyword supported in bump and get (issue #51) #69

Merged
merged 1 commit into from
Oct 12, 2022

Conversation

ranger6
Copy link
Collaborator

@ranger6 ranger6 commented May 19, 2022

The "diff" subcommand returns the keyword "prerelease" when it
is the pre-release part that differs. This keyword--different from
"prerel"--means using the output of "diff" is awkward to use as
input to "bump" or "get". This change creates a synonym for "prerel",
namely "prerelease" in the bump and get subcommands, fixing the
output/input awkwardness without breaking backwards compatibility.

The implementation is simple: normalize the part keyword to a
canonical string ("prerel"). This is completely transparent to
the external API.

The usage string and README have been updated accordingly.

The changes pass all the existing unit tests: no regression or
backward compatibility issues. Two new test cases added to cover
use of this alternative keyword.

When released, these changes require bumping semver's minor version.
As a placeholder, the version string has been set to 3.4.0.

README.md Outdated
@@ -45,10 +45,10 @@ usage

```
Usage:
semver bump (major|minor|patch|release|prerel [<prerel>]|build <build>) <version>
semver bump (major|minor|patch|release|prerel|prerelease [<prerel>]|build <build>) <version>
Copy link
Owner

@fsaintjacques fsaintjacques May 26, 2022

Choose a reason for hiding this comment

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

Suggested change
semver bump (major|minor|patch|release|prerel|prerelease [<prerel>]|build <build>) <version>
semver bump (major|minor|patch|release|prerel [<prerel>]|prerelease [<prerel>]|build <build>) <version>

The prerel sub-command also supports an argument. I believe it's time to start breaking this single line into multiple lines.

semver bump major <version>
semver bump minor <version>
...
semver bump prerel [<prerel>] <version>

This should improve readability.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agreed. Will break up this command. Try to make clear that the sub-commands "prerel" and "prerelease" are identical (just synonyms).

"bump release" removes any PRERELEASE or BUILD parts.
subsequent parts. "bump prerel" (or its synonym "bump prerelease")
sets the PRERELEASE part and removes any BUILD part. A trailing dot
in the <prerel> argument introduces an incrementing numeric field
Copy link
Owner

Choose a reason for hiding this comment

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

Is the prerel argument mandatory? If so, the usage should reflect that (remove the square bracket).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The argument to "prerel"/"prerelease" is indeed optional.

The "diff" subcommand returns the keyword "prerelease" when it
is the pre-release part that differs. This keyword--different from
"prerel"--means using the output of "diff" is awkward to use as
input to "bump" or "get".  This change creates a synonym for "prerel",
namely "prerelease" in the bump and get subcommands, fixing the
output/input awkwardness without breaking backwards compatibility.

The implementation is simple: normalize the part keyword to a
canonical string ("prerel").  This is completely transparent to
the external API.

The README and usage string have been updated accordingly.

The README and usage string now list each subcommand on a separate
line for readability: the previous list of subcommand choices being
too long.

The order of the subcommands listed has been changed: the two
"complex" subcommands ("bump" and "get") appear ahead of the
others.

The changes pass all the existing unit tests: no regression or
backward compatibility issues. Two new test cases added to cover
use of this alternative keyword.

When released, these changes require bumping semver's minor version.
As a placeholder, the version string has been set to 3.4.0.
@ranger6
Copy link
Collaborator Author

ranger6 commented Jun 14, 2022

Rebased and squashed PR to address the comments raised above. When the PR is accepted, it should be tagged 3.4.0 and released.

@ranger6
Copy link
Collaborator Author

ranger6 commented Oct 12, 2022

Since we've had a bit of new activity on this project lately (including a new PR), I'll take this opportunity to remind our owner that this PR has been ready to go for quite some time and the concerns in the comments above have been addressed. Ready to merge? (I'd do it myself, but I don't like accepting my own work).

@fsaintjacques
Copy link
Owner

@ranger6, I trust your judgement to merge your own (and others!) PRs. Woud you like that we move the repository to a non-personal organisation where you could be owner?

@fsaintjacques fsaintjacques merged commit 837ad91 into fsaintjacques:master Oct 12, 2022
@ranger6
Copy link
Collaborator Author

ranger6 commented Oct 12, 2022 via email

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