-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Expand the functionality of uv version --bump to support pre-releases
#13578
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
Conversation
| assert_eq!(version.to_string().as_str(), "5!2.0.0.0+local"); | ||
| version.bump(BumpCommand::BumpRelease { index: 0 }); | ||
| assert_eq!(version.to_string().as_str(), "5!3.0.0.0+local"); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These chunks of the PR should ideally cover any corner case you might want to ask about.
|
"Did you consider a |
|
For the sake of getting an intrusive thought out of my head: there are several times where I said "maybe you should be able to express |
|
Currently, there can be negative version bumps, where bumping |
konstin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code looks good, but I'm concerned by how easy it is to decrease the version accidentally, the version is decreased when using a prerelease bump on stable version by default.
|
I've pushed up a separate commit that adds an |
|
Updated cli docs a bit |
5a6c67d to
035a46a
Compare
035a46a to
2fddfa7
Compare
BurntSushi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work!!! I wish this was in Cargo. :-)
crates/uv-cli/src/lib.rs
Outdated
| Patch, | ||
| /// Make the version stable (1.2.3b4.post5.dev6 => 1.2.3) | ||
| /// | ||
| /// This intentionally clears `.postN` and preserves `+local` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might mention here that this could decrease the version number, and therefore, it's common to use this in conjunction with another --bump flag like --bump patch.
| release and the prerelease: | ||
|
|
||
| ```console | ||
| $ uv version --bump patch --bump beta |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting. I presume there was discussion on this, but I'm on my phone so just want to flag it. Why is the patch bump needed too? We fail without it?
(I'll also just read the rest of the pull request when I'm home)
docs/guides/package.md
Outdated
|
|
||
| !!! Note | ||
|
|
||
| If you only bump the prerelease here it will actually decrease the current version. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would say "it would decrease the current version" not "it will" if you're going to error.
|
Would it be possible to include a functionality of bumping the version in certain files, like commitizen does? It also allows to look for what lines to change according to a per-file regex |
I've certainly always found those kinds of features useful in similar commands, although it would require non-trivial design work we're definitely not doing in this PR. |
|
When will this be merged? |
|
When it's done — we're a small team balancing a lot of work! |
c5e8a6f to
78c50e3
Compare
uv version --bump uv version --bump to support pre-releases
|
I wonder if it wouldn't have been better to introduce a new argument instead of allowing repeated
And this would result in an error:
This would remove confusing behavior such as
Instead of EDIT2: one problem with the name "pre-bump" is that |
We just ban this now.
We removed this, it's not worth the complexity. |
|
Okay, that doesn't sound too bad then. It just seems to me that this commandline interface is "too weakly typed" in the sense that the "signature" seems to allow many things that will actually give a runtime error. |
|
I think the things that will give a runtime error are pretty weird for a user to do though, wouldn't you think? |
|
That's a good point, yes. |
This adds
alpha,beta,rc,stable,post, anddevmodes touv version --bump.The components that
--bumpaccepts are ordered as follows:Bumping a component "clears" all lesser component (
alpha,beta, andrcall overwrite each other):--bump minoron1.2.3a4.post5.dev6=>1.3.0--bump alphaon1.2.3a4.post5.dev6=>1.2.3a5--bump devon1.2.3a4.post5.dev6=>1.2.3a4.post5.dev7In addition,
--bumpcan now be repeated. The primary motivation of this is "bump stable version and also enter a prerelease", but it technically lets you express other things if you want them:--bump patch --bump alphaon1.2.3=>1.2.4a1("bump patch version and go to alpha 1")--bump minor --bump patchon1.2.3=>1.3.1("bump minor version and got to patch 1")--bump minor --bump minoron1.2.3=>1.4.0("bump minor version twice")The
--bumpflags are sorted by their priority, so that you don't need to remember the priority yourself. This ordering is the only "useful" one that preserves every--bumpyou passed, so there's no concern about loss of expressiveness. For instance--bump minor --bump majorwould just be--bump majorif we didn't sort, as the major bump clears the minor version. The ordering ofbetaafteralphameans--bump alpha --bump betawill just result in beta 1; this is the one case where a bump request will effectively get overwritten.The
stablemode "bumps to the next stable release", clearing the pre (alpha,beta,rc),dev, andpostcomponents from a version (1.2.3a4.post5.dev6=>1.2.3). The choice to clearposthere is a bit odd, in that1.2.3.post4=>1.2.3is actually a version decrease, but I think this gives a more intuitive model (as preservingpost5in the previous example is definitely wrong), and also post-releases are extremely obscure so probably no one will notice. In the cases where this behaviour isn't useful, you probably wanted to pass--bump patchor something anyway which should definitely clear thepost5(putting it another way: the only cases where--bump stablehas dubious behaviour is when you wanted it to do a noop, which, is a command you could have just not written at all).In all cases we preserve the "epoch" and "local" components of a version, so the
7!and+localin7!1.2.3+localwill never be modified by--bump(you can use the raw version set mode if you want to touch those). The preservation oflocalis another slightly odd choice, but it's a really obscure feature (so again it mostly won't come up) and when it's used it seems to mostly be used for referring to variant releases, in which case preserving it tends to be correct.Fixes #13223