RustySecrets icon indicating copy to clipboard operation
RustySecrets copied to clipboard

Add clippy to Travis and remove unnecessary code

Open romac opened this issue 7 years ago • 5 comments

On hold. See https://github.com/SpinResearch/RustySecrets/pull/66#issuecomment-391170134.

romac avatar May 16 '18 17:05 romac

Maybe wait on this until a unified Travis testing strategy is figured out for all the RustySecrets related repositories (see https://github.com/SpinResearch/merkle.rs/pull/36#discussion_r190078423).

psivesely avatar May 22 '18 23:05 psivesely

@nvesely Agreed.

romac avatar May 23 '18 08:05 romac

I'm working on a solution I'm testing in github.com/nvesely/winternitz. The idea is that a Rust nightly version is pinned as well as a clippy version. As @afck pointed out rustup component add rustfmt-preview will effectively pin a rustfmt version (see https://github.com/SpinResearch/merkle.rs/pull/36#discussion_r190145420).

Additionally, commands are added to the Makefile so that one neither needs to remember/ type the correct version of Rust nightly or all the options one must pass to clippy or rustfmt. Commands are added both to install what's necessary to format and lint and to run these tools with the set of options to match Travis/ our expectations.

One shouldn't need to update the pinned versions more than every couple of months or so and it should be possible to store the values only in the Makefile (and have Travis read them from there).

I think this solves a few problems:

  • Travis builds should never suddenly break on master because, e.g., rustfmt updated.
  • PR diffs will be cleaner because no code that is not new will be present.
  • The inconveniences and potential problems mentioned in https://github.com/SpinResearch/merkle.rs/pull/36#discussion_r189174673 are alleviated.

psivesely avatar May 23 '18 10:05 psivesely

Clippy can now also be installed via rustup (instead of cargo install), so you don't have to manually match Clippy versions to the corresponding Rust nightlies anymore. And it will even be part of one of the upcoming stable versions! :tada: https://internals.rust-lang.org/t/clippy-is-available-as-a-rustup-component/7967

afck avatar Aug 13 '18 20:08 afck

That simplifies things a lot. Ideally our CI is simply not as complicated as some of the ideas we were discussing a month or two back :smiley_cat:

psivesely avatar Aug 21 '18 23:08 psivesely