Closed kbdharun closed 1 year ago
Hi :) These changes seem nice, but they make it a bit intransparent as to what versions are used. For example, I dont see which version of the rust compiler is running (and we also want to build with two specific versions: the msrv and stable). About the OS update: not sure either, but should probably be fine
Hi :) These changes seem nice, but they make it a bit intransparent as to what versions are used. For example, I don't see which version of the rust compiler is running (and we also want to build with two specific versions: the msrv and stable).
Sorry for the delay, I wasn't feeling well. I have updated the workflow now to support multiple versions of Rust, a test action run can be found here [Note: the failed actions are because I am running it in a fork, it should run fine here, also by the way can you approve the workflow run]
About the OS update: not sure either, but it should probably be fine
Yep, it is better to use the latest stable versions to have faster CIs and additional features. Let me know If you guys need me to revert it.
Hi, I think it is still not correct: You have added the line back in that starts more CI runs with different values for the rust
variable, but this variable is never used (note the use of ${{ matrix.rust }}
before when installing the toolchain). Currently, it is still the preinstalled version of Rust that is used, which is not what we want for the reasons I wrote before.
I was unaware that actions-rs is not maintained anymore, so switching to something else seems like a good idea. From what I can tell from https://github.com/actions-rs/toolchain/issues/216 and for example https://github.com/jonhoo/rust-ci-conf/commit/362696ab8007ef1a4779885a398286856cacf555 the world is moving to dtolnay's action, so I think going with that would be a sane move :)
Note: the failed actions are because I am running it in a fork
Actually, I think that you forgot to add the checkout
action back in in the two jobs that are failing as both fail because of a missing Cargo.toml
file :)
About using latest os, I was wondering if we would maybe want to stay with the older versions so that we don't miss an incompatibility with for example windows 10, ubuntu 22.04 or so. @dbrgn what do you think about this?
Hi, I think it is still not correct: You have added the line back in that starts more CI runs with different values for the
rust
variable, but this variable is never used (note the use of${{ matrix.rust }}
before when installing the toolchain). Currently, it is still the preinstalled version of Rust that is used, which is not what we want for the reasons I wrote before.
Noted
I was unaware that actions-rs is not maintained anymore, so switching to something else seems like a good idea. From what I can tell from actions-rs/toolchain#216 and for example jonhoo/rust-ci-conf@362696a the world is moving to dtolnay's action, so I think going with that would be a sane move :)
Agreed, I will try to update this PR to https://github.com/dtolnay/rust-toolchain as even with matrix jobs it is possible for some old versions of packages to be dropped from runner images i.e https://github.com/actions/runner-images/blob/main/images/linux/Ubuntu2204-Readme.md.
Note: the failed actions are because I am running it in a fork
Actually, I think that you forgot to add the
checkout
action back in in the two jobs that are failing as both fail because of a missingCargo.toml
file :)
Oops, just now noticed it will fix it.
About using latest os, I was wondering if we would maybe want to stay with the older versions so that we don't miss an incompatibility with for example windows 10, ubuntu 22.04 or so. @dbrgn what do you think about this?
Regarding this, the official GitHub runner doesn't support Windows 10 runners while the ubuntu-latest
runner is the latest LTS Ubuntu version which is 22.04 now. Do note if you need I could add Ubuntu 20.04 to tests but the runner will get removed next year when 24.04 gets released as GitHub only keeps 2 LTS versions of Ubuntu runners at a time. An alternative is running the job in a container (as GitHub self-hosted runners aren't advisable for public projects)
Edit. Done updated the CI to the suggested toolchain action, and all jobs have succeeded https://github.com/kbdharun/tealdeer/actions/runs/5124671837 (as requested I added an older supported runner for Ubuntu 20.04)
Done, I think it is GTG now.
This looks good to me, let's see if CI runs through! If yes, I think we can merge this.
Thanks for your work and your patience, @kbdharun and @niklasmohrin 🙂
Changes
actions/checkout
version tov3
(Node 16).actions/upload-artifact
version tov3
(Node 16).actions/download-artifact
version tov3
(Node 16).