-
Notifications
You must be signed in to change notification settings - Fork 109
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
Enable clippy #182
Enable clippy #182
Conversation
This is based on a wrong branch at the moment but useful to test CI. After #181 we can rebase it onto main. |
0a829cf
to
73c04bf
Compare
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.
@allada @chrisstaite-menlo It works 🥳
Reviewable status: 0 of 8 files reviewed, all discussions resolved
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.
Reviewable status: 0 of 1 files reviewed, all discussions resolved (waiting on @allada)
.bazelrc
line 39 at r1 (raw file):
test --output_groups=+clippy_checks # Convenience config to build a single target with clippy enabled. Use as:
Not sure we really need this anymore, but I guess it could be useful to A/B test using clippy and no clippy in some scenarios?
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.
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @aaronmondal)
.bazelrc
line 39 at r1 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
Not sure we really need this anymore, but I guess it could be useful to A/B test using clippy and no clippy in some scenarios?
eeeh, I would remove it. In reality the opposite is more likely, people are more likely to want to disable clippy when running tests. If you really want to do something here, I'd add noclippy
.
After this change it is a hard error to commit anything that doesn't conform to Clippy's default warning set.
73c04bf
to
11a200e
Compare
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.
Reviewable status: 0 of 1 files reviewed, all discussions resolved (waiting on @allada)
.bazelrc
line 39 at r1 (raw file):
Previously, allada (Nathan (Blaise) Bruer) wrote…
eeeh, I would remove it. In reality the opposite is more likely, people are more likely to want to disable clippy when running tests. If you really want to do something here, I'd add
noclippy
.
Yeah seems kinda unnecessary. I'll just remove it. We can always add more configs later if we really feel like we need them 😇
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.
Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @aaronmondal)
After this change it is a hard error to commit anything that doesn't
conform to Clippy's default warning set.
This change is