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

ci(periodic): Use cargo action for clippy #349

Merged
merged 1 commit into from
Jan 14, 2021

Conversation

flub
Copy link
Contributor

@flub flub commented Jan 14, 2021

Giving up on this.

Run clippy with the cargo action. The annotations from the clippy
action end up in the wrong place and github can't figure this out.
Running the cargo action still annotates via stdout plus it doesn't
run the actualy command with json output so humans can read it anyway.

#skip-changelog

Run clippy with the cargo action.  The annotations from the clippy
action end up in the wrong place and github can't figure this out.
Running the cargo action still annotates via stdout plus it doesn't
run the actualy command with json output so humans can read it anyway.
@flub flub requested a review from a team January 14, 2021 09:37
@flub
Copy link
Contributor Author

flub commented Jan 14, 2021

@Swatinem
Copy link
Member

I’m 👍 on this, but I guess @jan-auer will disagree.

@Swatinem
Copy link
Member

BTW, I think this is the ICE that we hit on nightly: rust-lang/rust#81000

@flub flub merged commit b0d8fb3 into master Jan 14, 2021
@flub flub deleted the ci/periodic-clippy-cargo-action branch January 14, 2021 15:09
@jan-auer
Copy link
Member

I guess @jan-auer will disagree.

Not necessarily, but please point me in the right direction. Previously, clippy violations were annotated on the source code. This was a super useful feature, as it showed the suggested fix in line with the diff. Where is this now?

Separately, if you know I disagree, could we please hold off merging until we have reached agreement?

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