-
Notifications
You must be signed in to change notification settings - Fork 150
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
feat: MVP for cargo set-version
#482
Conversation
11020d5
to
40aa520
Compare
b7d3295
to
db3cf69
Compare
Sorry for the delay, I'll take a look this week. |
Thanks! |
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 implementation looks good.
Naming bikeshed:
what do you think of using cargo bump
instead of cargo set-version
? E.g. cargo bump 1.0.0
or cargo bump minor
.
README.md
Outdated
release, rc, beta, alpha] | ||
--exclude <exclude>... Crates to exclude and not modify | ||
--manifest-path <path> Path to the manifest to upgrade | ||
-m, --metadata <metadata> Version metadata |
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.
is that what goes after +
https://docs.rs/semver/1.0.4/semver/struct.BuildMetadata.html
would be nice to provide an example
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.
Good idea. I gave an example of a use case and in the README pointed to that doc since it does such a good job describing it and how one might use it. Figure it'd be better to link than having to maintain our own copy.
@@ -43,6 +43,11 @@ name = "cargo-upgrade" | |||
path = "src/bin/upgrade/main.rs" | |||
required-features = ["upgrade"] | |||
|
|||
[[bin]] | |||
name = "cargo-set-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.
what do you think of using cargo bump instead of cargo set-version? E.g.
cargo bump 1.0.0
orcargo bump minor
.
I did bring up that the current name is what drove me to not overload the version field with relative bumps but to push that out to --bump
. The side benefit is error reporting is more complicated when mixing fields.
Personally, I have no real preference within the scope of cargo-edit
. I do wonder what would be the more appropriate name if we upstream this into cargo
. I would suspect a more explicit name (ie has version
in it).
I thought I'd look around for other tools
- yarn calls it
version
- Same with lerna
- The canonical python tool is bumpversion
- Some other misc tools
- While CalVer isn't good for libs, wonder if we should support it for bins. Maybe enough of a corner case to leave it to people to give the version specifically.
The name is not `version` because `cargo version` reports cargo's version number. The UI and implementation is lifted from `cargo-release` Differences with `cargo-release`: - Relative version changes is behind `--bump` rather than overloading the positional argument - Motivation: It didn't jive with `set-` in the name - Side benefit: Better error reporting Differences with killercup#414: - `--bump <level>` vs `--<level>` - `--bump` scales better - This PR supports `--metadata`, `--exclude` Required future work - Update workspace dependents when modifying versions Possible future work - `--at-least` flag to ignore downgrades, rather than error - A flag to upgrade everything to the highest requested version (keep things in lockstep with `--bump patch`) - A flag to upgrade min requirements on dependents (rather than just updating if semver is broken) - `--pre` flag so you can do `--bump major --pre alpha` (even `1.0 --pre alpha` since I always forget that syntax)
The name is not
version
becausecargo version
reports cargo'sversion number.
The UI and implementation are lifted from
cargo-release
Differences with
cargo-release
:--bump
rather than overloadingthe positional argument
set-
in the nameDifferences with #414:
--bump <level>
vs--<level>
--bump
scales better--metadata
,--exclude
Required future work
Possible future work
--at-least
flag to ignore downgrades, rather than errorthings in lockstep with
--bump patch
)updating if semver is broken)
--pre
flag so you can do--bump major --pre alpha
(even1.0 --pre alpha
since I always forget that syntax)Closes #338