search

rust-itertools / itertools

Extra iterator adaptors, iterator methods, free functions, and macros.
https://docs.rs/itertools/
Apache License 2.0
2.69k stars 303 forks source link

pin the toolchain version used by clippy #846

Closed danieleades closed 7 months ago

danieleades commented 7 months ago

see discussion in #618

Philippe-Cholet commented 7 months ago

I was working on this but was kinda blocked (I mostly discover dependabot to be honest), so thanks.

What bug me about this kind of change is #758 that we did not want.

Are you saying that with: toolchain: 1.43.1 will make dependabot ignore it? That sounds so simple as I was looking in another direction to solve this (use "ignore" option).

danieleades commented 7 months ago

I was working on this but was kinda blocked (I mostly discover dependabot to be honest), so thanks.

What bug me about this kind of change is #758 that we did not want.

Are you saying that with: toolchain: 1.43.1 will make dependabot ignore it? That sounds so simple as I was looking in another direction to solve this (use "ignore" option).

You certainly can configure dependabot to ignore the whole action, but that would also prevent it from bumping the pinned clippy toolchain version.

Dependabot doesn't parse the toolchain config, only the action version, so this should work fine (though it's untested).

I can probably test it on a fork to confirm it works the way I expect

Philippe-Cholet commented 7 months ago

Ok that seems good. Another thing that bothered me in #758 was that it suggested to update the version to "1.80.0" while I thought it would have suggested "1.74.0" (or whatever appropriate version at the time it wrote it).

danieleades commented 7 months ago

Ok that seems good. Another thing that bothered me in #758 was that it suggested to update the version to "1.80.0" while I thought it would have suggested "1.74.0" (or whatever appropriate version at the time it wrote it).

I missed that, that could be a problem. Dependabot here is not doing anything clever to check the latest stable version, it's just checking the versions of the GitHub action that have been published. Perhaps these higher numbers are aliases for nightly releases?

Philippe-Cholet commented 7 months ago

This is intentional but it's convenient ahead-of-time PR. I'm having a hard time finding a Rust repo pinning clippy (and using Dependabot).

mightyiam commented 7 months ago

For my taste, this is too many things in one PR.

danieleades commented 7 months ago

I'm having a hard time finding a Rust repo pinning clippy (and using Dependabot).

pinning the version of linters is a general pattern, not limited to the Rust ecosystem

danieleades commented 7 months ago

For my taste, this is too many things in one PR.

      - uses: dtolnay/rust-toolchain@master
        with:
          toolchain: stable
          components: rustfmt

to

      - uses: dtolnay/rust-toolchain@stable
        with:
          components: rustfmt

is not strictly required, but trivial. All other changes are required.

danieleades commented 7 months ago

This is https://github.com/dtolnay/rust-toolchain/issues/43 but it's convenient ahead-of-time PR.

i think this might be deal-breaker unfortunately

Philippe-Cholet commented 7 months ago

i think this might be deal-breaker unfortunately

I agree. I think we are gonna stick with stable clippy and handle rare breakage as soon as it appears.

PS: There is not that many things in that PR.

Philippe-Cholet commented 7 months ago

Thank you anyway.