-
Notifications
You must be signed in to change notification settings - Fork 1.6k
only require rustfmt and update_lints for version bumps #1538
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
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.
❤️
- Write a changelog entry. | ||
- Commit `./Cargo.toml`, `./clippy_lints/Cargo.toml` and `./CHANGELOG.md`. | ||
- Review and commit all changed files |
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 prefer to have one commit just for the version bump itself,that is the Cargo.toml
s and the changelog.
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 changed the script to do this for us
pre_publish.sh
Outdated
set -e | ||
|
||
cd clippy_lints && cargo fmt && cd .. | ||
cargo fmt |
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.
Can it add --write-mode=overwrite
if the repo is clean? I hate those .bk files
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.
done
pre_publish.sh
Outdated
|
||
# add all changed files | ||
git add . | ||
git commit -m "version bump" |
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 definitely needs to contain the version then. Also, this fails rules 3 and 5.
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.
we tag it later, do we really need the version in there?
any further complaints? or can we merge this? |
should we move dogfood tests to version bumps, too? |
No. They are good to see the effect of a new lint on a real codebase. See #1556 for example, dogfood shows quite a few nice examples of what the lint does. |
I'm also bullish in dogfooding. |
Yay! \o/ |
fixes #1487