-
Notifications
You must be signed in to change notification settings - Fork 594
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
A bunch of Clippy fixes #2270
A bunch of Clippy fixes #2270
Conversation
150e88d
to
22ae1ca
Compare
Soo many opportunities for |
@@ -1,6 +1,6 @@ | |||
[package] | |||
name = "image" | |||
version = "0.25.1" | |||
version = "0.25.2" |
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 PR description says bumping the version field was needed for semver-checks? Might be worth seeing if we can avoid that so an accidental cargo publish
with the repository checked out doesn't result in a new version upload to crates.io until we're actually ready to release
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.
This is because the semver check saw a change that required a minor version bump, and would continue to complain about it until the version has been bumped accordingly.
I guess the CI could be changed to look only for semver-major changes.
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.
There's no harm bumping by two minor versions in a single release, so I think the risk is at least low. At least lower than missing some of these warnings, I imagine. Maybe we can have a look at our release workflow instead, should be simple to compare against the latest published tag and do the version bump conditionally. I think this diff could also serve as one signal for when a release should be done.
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 failure mode I'm afraid of is me trying to publish a different crate but accidentally running cargo publish
in the wrong terminal window. That'll push whatever branch I happen to have checked out as the next image
release even if it just includes some half-baked in progress changes.
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.
Created #2277 which seems to fix it
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.
Maybe dev version of the crate could have publish = false
set?
or add ./cargo/config.toml
that somehow breaks the registry configuration?
[registry]
default = "…"
you could also remove your auth token from the default config, and set CARGO_REGISTRY_TOKEN
only via some release wrapper with "are you totally sure?"
size_of will be in prelude (it is in nightlies already).