search

ssalonen / libcec-sys

FFI bindings for the libcec
GNU General Public License v2.0
2 stars 3 forks source link

Better CI #19

Closed ok-nick closed 1 year ago

ok-nick commented 2 years ago

Removes redundancies and updates actions.

cargo fmt is extremely fast, if it fails, clippy fails. It makes sense to combine them into one job.

ssalonen commented 2 years ago

Thanks!

Additional improvement: I can see that with checlout@v3 (or anything newer than v2.4.0, see https://github.com/actions/checkout/commit/ec3a7ce113134d7a93b817d10a8272cb61118579) we do not need that custom git checkout for submodules. We should be able to remove both by specifying recursive submodule checkout with the checkout action

git -c "url.https://github.com/.insteadOf=git@github.com:" submodule update And git submodule recursive

  1. Howcome it is not necessary to install / specify rust toolchain within the lint job? Even the latest template I have used as base still has it. It seems also the examples in actions-rs/cargo have it.

EDIT: Answering myself: it seems that the ubuntu-latest "virtual environment" contains certain version of cargo and other tools (eg cmake): https://github.com/actions/virtual-environments/blob/main/images/linux/Ubuntu2004-Readme.md

  1. I can see you opted for raw cargo command (instead of cargo action, https://github.com/actions-rs/cargo). Was that intentional? Please see "Why would you want to use this Action instead" I'm actions-rs/cargo docs

  2. The new template uses caching, https://github.com/Swatinem/rust-cache. Would it bring some advantages here as well?

ok-nick commented 2 years ago

I'm not sure, GitHub Action hosted runners are always updated with the latest. Maybe I'll leave an issue on their repo?

The cargo action has a section in the readme for "use cases." It's not necessary unless you need something like cross.

It would also be a good idea to add the cargo cache action and I removed the -D warnings flag from clippy accidentally.

ssalonen commented 2 years ago

Re point 1: I am good utilizing the cargo clippy and other default versions for linting. With build I would prefer more explicit choice of tools, as to communicate tested/supported rustc.

Regarding cargo action (point 2), you are correct that is not really required. However, it might bring some QoL improvements: they are saying that "Warnings and errors issued by cargo will be displayed in GitHub UI". Is this useful?

ssalonen commented 1 year ago

Closing this one in favor of #34 - same as this one but rebased

Thanks @ok-nick !