-
Notifications
You must be signed in to change notification settings - Fork 601
add a note about cargo release config #263
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
Pull Request Test Coverage Report for Build 275556644
💛 - Coveralls |
Cargo.toml
Outdated
[package.metadata.release] | ||
# We want to ensure we don't publish via `cargo release` since Actions | ||
# handles this for us. | ||
disable-publish = true |
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.
Thanks. May I suggest:
# Instruct `cargo release` to not run `cargo publish` locally:
# https://github.com/sunng87/cargo-release/blob/master/docs/reference.md#config-fields
# See docs/releasing.md for details.
BTW it appears that |
I didn't explicitly call that out (I should have) but I assumed that that would be a desirable side effect. I wouldn't suggest having it not push tags because then we might accidentally push the wrong tags if we do so manually. So there is a benefit to encapsulating that responsibility. However it could be surprising that running the command will in fact create a Crates release. |
The problem is that the docs make it seem that pushing is a separate step. Frankly I don't see the problem of "accidentally push[ing] the wrong tags", but as cargo-release defaults to doing everything in one step, I think it's fine to do so --- as long as we don't have contradicting documentation. Also, in my opinion, if we decide we want to release in one command, it shouldn't be documented as |
It's pretty easy to push local tags that you may not mean to push. That might not be desirable and the sub command should help avoid that. Feel free to make any edits you see fit. 🙂 |
The tags are named vX.Y, and we'll be 0.x for a while, so limiting the publish-crate action to run v0 tags seems good enough to avoid accidental publishes.
I had a go at it, what do you think? |
No description provided.