Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Get rid of clippy remainings. #7355

Merged
merged 2 commits into from Dec 22, 2017
Merged

Get rid of clippy remainings. #7355

merged 2 commits into from Dec 22, 2017

Conversation

tomusdrw
Copy link
Collaborator

We don't really use this feature any more.

I tried running clippy as cargo plugin, but unfortunatelly it failed to compile Parity on latest nightly.

@tomusdrw tomusdrw added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Dec 21, 2017
@svyatonik
Copy link
Collaborator

@tomusdrw tomusdrw added A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. and removed A0-pleasereview 🤓 Pull request needs code review. labels Dec 21, 2017
@tomusdrw tomusdrw added A0-pleasereview 🤓 Pull request needs code review. and removed A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. labels Dec 21, 2017
@tomusdrw
Copy link
Collaborator Author

Good catch. Forgot to update sed params for those :)

@svyatonik svyatonik added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Dec 22, 2017

// Clippy settings
// Most of the time much more readable
#![cfg_attr(feature="dev", allow(needless_range_loop))]
Copy link
Collaborator

Choose a reason for hiding this comment

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

if we want to run clippy as a plugin, we should not remove those, but only rename to:

#[cfg_attr(feature="cargo-clippy", allow(needless_range_loop))]

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I tried running cargo-clipp as a plugin but it didn't work (I think parity failed to compile on nightly). Let's look at this from a fresh perspective when it works, some of the things here might not be valid any more, I would prefer to fix the code at some point rather than disable lints.
What do you think?

Copy link
Collaborator

Choose a reason for hiding this comment

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

fair enough ;)

@debris debris added A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. A8-looksgood 🦄 Pull request is reviewed well. and removed A8-looksgood 🦄 Pull request is reviewed well. A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. labels Dec 22, 2017
@debris debris merged commit 7c24d06 into master Dec 22, 2017
@debris debris deleted the td-clippy branch December 22, 2017 11:09
@5chdn 5chdn added this to the 1.9 milestone Jan 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants