Conversation
thomaseizinger
left a comment
thomaseizinger
left a comment
There was a problem hiding this comment.
Thanks! I am in favor of this.
I don't know why we are referencing actions by hash instead of version. @mxinden Do you know?
| - name: Run cargo clippy | ||
| uses: actions-rs/cargo@844f36862e911db73fe0815f00a4a2602c279505 # v1.0.3 | ||
| with: | ||
| command: custom-clippy # cargo alias to allow reuse of config locally |
There was a problem hiding this comment.
We are running an alias here, please retain that (with the comment!).
There was a problem hiding this comment.
But why are you running an alias here? I don't quite understand the assignment.
There was a problem hiding this comment.
The alias is a workaround for clippy still not having a config file.
It is defined here: https://github.com/libp2p/rust-libp2p/blob/master/.cargo/config.toml#L3
This allows us to allow certain lints across the codebase without having to duplicate this in a bunch of places :)
Reasoning is here:
I don't have a strong opinion here. Maybe @galargh does? |
|
Yes, exactly! Pinning actions by hash is a mitigation for a potential supply chain attack. We try to do that for all actions that are neither from PL orgs nor https://github.com/actions. |
|
Okay, thanks for clarifying! Let's go back to commit hashes then please @umgefahren! Plus, #2917 (comment) needs to be addressed before we can merge this! |
thomaseizinger
left a comment
thomaseizinger
left a comment
There was a problem hiding this comment.
Thank you!
I think the original idea of the PR - to get rid of actions-rs/toolchain is good. To stay focused on that, can we revert the other changes to what they were? I think the old commit hashes been set exactly to the hash of the releases before and now we are pointing to some commit on master.
| - name: Run cargo clippy | ||
| uses: actions-rs/cargo@844f36862e911db73fe0815f00a4a2602c279505 # v1.0.3 | ||
| with: | ||
| command: custom-clippy # cargo alias to allow reuse of config locally |
There was a problem hiding this comment.
The alias is a workaround for clippy still not having a config file.
It is defined here: https://github.com/libp2p/rust-libp2p/blob/master/.cargo/config.toml#L3
This allows us to allow certain lints across the codebase without having to duplicate this in a bunch of places :)
| - uses: actions/checkout@v3 | ||
|
|
||
| - uses: Swatinem/rust-cache@6720f05bc48b77f96918929a9019fb2203ff71f8 # v2.0.0 | ||
| - uses: Swatinem/rust-cache@76686c56f2b581d1bb5bda44b51f7e24bd9b8b8e |
There was a problem hiding this comment.
As far as I can see, this hash is not yet part of a released version. I'd prefer if we just not touch this line at all and revert back to exactly what we had :)
| access_token: ${{ github.token }} | ||
|
|
||
| - name: Install Protoc | ||
| uses: arduino/setup-protoc@v1 |
There was a problem hiding this comment.
It is my understanding that v1 would actually correspond to arduino/setup-protoc@64c0c85.
| steps: | ||
|
|
||
| - name: Cancel Previous Runs | ||
| uses: styfle/cancel-workflow-action@bb6001c4ea612bf59c3abfc4756fbceee4f870c7 # 0.10.0 |
There was a problem hiding this comment.
Let's revert this too please.
|
I will take another go at this later. Closing for now. |
4 participants
Description
I removed some steps from the CI to improve the peformance. This is possible, because GitHub Actions runners now come with Rust support.
Links to any relevant issues
Origin: #2900