Skip to content
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

Ensure clippy is run for all targets and switch to clippy-preview on stable #1493

Merged
merged 1 commit into from
Oct 8, 2018

Conversation

jtgeibel
Copy link
Member

@jtgeibel jtgeibel commented Sep 20, 2018

--all-targets is now passed to clippy on CI. Temporarily,
-- -A renamed_and_removed_lints is passed as well. This can be
removed once the clippy::* attribute syntax is available in stable.

Additionally, clippy-preview is now installed and run on stable.
We were previously running clippy on an old nightly from late June and
no new warnings (except the one noted above) are raised on nightly so
switching to the -preview component on stable shouldn't introduce
churn. Also, unlike with rustfmt, we can fix and merge any clippy
recommendations before they hit the stable channel, so we don't have to
sync with new releases.

@sgrif
Copy link
Contributor

sgrif commented Sep 25, 2018

If clippy shipped on stable is recommending something that is not possible on stable, we should open an issue on clippy for that.

@jtgeibel
Copy link
Member Author

It looks like this has been reported upstream in rust-lang/rust#54406. It doesn't affect stable yet, but does affect beta and nightly. I believe the intent is to stabilize this in time for the 2018 edition (see rust-lang/rust#44690 (comment)) so hopefully the allow attribute isn't needed for long.

@jtgeibel jtgeibel changed the title Allow renamed lints and enable clippy-preview on stable Ensure clippy is run for all targets and switch to clippy-preview on stable Oct 6, 2018
@jtgeibel
Copy link
Member Author

jtgeibel commented Oct 6, 2018

I've reworked the implementation and updated the PR description. --all-targets is now passed to clippy and a few minor issues were addressed in the tests. -- -A renamed_and_removed_lints is now applied only to the clippy command, since these warnings don't affect other build profiles.

…stable

`--all-targets` is now passed to clippy on CI.  Temporarily,
`-- -A renamed_and_removed_lints` is passed as well.  This can be
removed once the clippy::* attribute syntax is available in stable.

Additionally, clippy-preview is now installed and run on stable.
We were previously running clippy on an old nightly from late June and
no new warnings (except the one noted above) are raised on nightly so
switching to the -preview component on stable shouldn't introduce
churn.  Also, unlike with rustfmt, we can fix and merge any clippy
recommendations before they hit the stable channel, so we don't have to
sync with new releases.
@carols10cents
Copy link
Member

Woohoo! I'm excited to not have to keep a particular nightly around anymore :P

bors: r+

bors-voyager bot added a commit that referenced this pull request Oct 8, 2018
1493: Ensure clippy is run for all targets and switch to clippy-preview on stable r=carols10cents a=jtgeibel

`--all-targets` is now passed to clippy on CI.  Temporarily,
`-- -A renamed_and_removed_lints` is passed as well.  This can be
removed once the clippy::* attribute syntax is available in stable.

Additionally, clippy-preview is now installed and run on stable.
We were previously running clippy on an old nightly from late June and
no new warnings (except the one noted above) are raised on nightly so
switching to the -preview component on stable shouldn't introduce
churn.  Also, unlike with rustfmt, we can fix and merge any clippy
recommendations before they hit the stable channel, so we don't have to
sync with new releases.

Co-authored-by: Justin Geibel <jtgeibel@gmail.com>
@bors-voyager
Copy link
Contributor

bors-voyager bot commented Oct 8, 2018

Build succeeded

@bors-voyager bors-voyager bot merged commit f6e34a1 into rust-lang:master Oct 8, 2018
@jtgeibel jtgeibel deleted the clippy-preview branch October 9, 2018 00:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants