Run clippy on a fixed version of rustc
epage opened this issue · 3 comments
By running clippy on a non-fixed version of rustc, CI will break as new clippy's introduce new checks. Having the CI fail on a new contributor can be discouraging and then once they realize it is a sporadic failure, it can give them the sense of the project being not well maintained.
To resolve this, I've found pinning the version of rustc to be the best approach.
I'm of two minds on this. On the one hand, I agree that it's unfortunate for newcomers to be met with clippy errors on code they didn't introduce, but at the same time I think it's important to keep clippy up-to-date, otherwise it is so easy to miss new true error cases that clippy identifies. Pinning to an older version means that we're losing out on improvements (and, yes, new warnings) from clippy, and authors now have to remember to bump their clippy version (which, my guess is, no-one will remember).
I think my vote here would still be to use clippy from stable, and then have clippy from beta with allow_fail. That way developers will generally be warned in advance if something will start failing, and will hopefully fix it before contributors run into it.
I strongly disagree. One of my guiding principles is that the CI should never be red, especially for new contributors.
Also, I suspect I put a lower value on new clippy warnings. I'm fine with the suggestion of tying clippy to minrust. It is rare for minrust to be more than a couple versions (I think I've seen one case of 15 releases back and one case that goes to rustc 1.0) and I've generally not seen new lints introduced that make me want to upgrade.
I think this gets pretty difficult to enforce, especially if we make minrust optional (#20). I generally keep minrust as low as I possibly can for my libraries, which would leave my clippy far behind. In extreme cases it might also lead to a situation where clippy locally tells me to change something, but if I do, clippy on CI will start complaining, which seems like an issue.
I think clippy from stable and allow_fail clippy from beta gives us a decent trade-off. New contributors won't generally see failing CI due to clippy (because the codebase is presumably clean on clippy stable), and developers will be warned early if upcoming clippy releases will break things for them.