Skip to content
This repository has been archived by the owner on Jan 22, 2025. It is now read-only.

Temporarily use stable clippy for redundant_clone #31692

Merged
merged 1 commit into from
May 17, 2023

Conversation

ryoqun
Copy link
Contributor

@ryoqun ryoqun commented May 17, 2023

Problem

I disabled rather useful clippy lint altogether at #31381 , causing some trouble around bp-ing. ref: #31690

Summary of Changes

Restore the functionary with somewhat degraded state as a band-aid fix for now..

namely, check will take extra minutes, but i think it's acceptable.

upstream bug: rust-lang/rust-clippy#10577

@ryoqun ryoqun requested review from CriesofCarrots and yihau May 17, 2023 12:03
@ryoqun ryoqun force-pushed the good-old-clippy branch 2 times, most recently from ec8d694 to 117ea40 Compare May 17, 2023 12:19
@ryoqun ryoqun force-pushed the good-old-clippy branch from 117ea40 to ffa26ea Compare May 17, 2023 12:32
@ryoqun
Copy link
Contributor Author

ryoqun commented May 17, 2023

see! this pr is working! seems #31690 wasn't merged yet (i did afterwards):

https://buildkite.com/solana-labs/solana/builds/95758#018829a7-7090-4ce1-8693-2ad0c0100f1c/1717-2374

image

@ryoqun
Copy link
Contributor Author

ryoqun commented May 17, 2023

seems ci is green, adding automerge. :)

i had some fruitful debugging session diving into rust/clippy:

rust-lang/rust-clippy#10577 (comment)

@ryoqun ryoqun added the automerge Merge this Pull Request automatically once CI passes label May 17, 2023
@yihau
Copy link
Contributor

yihau commented May 17, 2023

I think this one is good but I defer to Tyera. 🫶

Copy link
Contributor

@CriesofCarrots CriesofCarrots left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It isn't the best that we have to run clippy twice, but I guess I'm okay with the band-aid. Can you please create an issue linking to the clippy issue so that we can track when it's fixed and simplify this job again?

What's the argument for running nightly clippy everywhere, incidentally? The comment only refers to sdk. It would also be nice to document what nightly-only code we're actually using there.

@CriesofCarrots
Copy link
Contributor

(Thank you for jumping on this, btw!)

@mergify mergify bot merged commit bd8289e into solana-labs:master May 17, 2023
@ryoqun
Copy link
Contributor Author

ryoqun commented May 25, 2023

It isn't the best that we have to run clippy twice, but I guess I'm okay with the band-aid. Can you please create an issue linking to the clippy issue so that we can track when it's fixed and simplify this job again?

What's the argument for running nightly clippy everywhere, incidentally? The comment only refers to sdk. It would also be nice to document what nightly-only code we're actually using there.

beep beep (next in my loong task queue). I'm hearing!

@ryoqun
Copy link
Contributor Author

ryoqun commented May 26, 2023

It isn't the best that we have to run clippy twice, but I guess I'm okay with the band-aid. Can you please create an issue linking to the clippy issue so that we can track when it's fixed and simplify this job again?
What's the argument for running nightly clippy everywhere, incidentally? The comment only refers to sdk. It would also be nice to document what nightly-only code we're actually using there.

beep beep (next in my loong task queue). I'm hearing!

hi, I'm reporting back!

... Can you please create an issue linking to the clippy issue so that we can track when it's fixed and simplify this job again?

done: #31834

What's the argument for running nightly clippy everywhere, incidentally? The comment only refers to sdk. It would also be nice to document what nightly-only code we're actually using there.

done: #31833

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
automerge Merge this Pull Request automatically once CI passes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants