Skip to content

CI: add check+clippy+audit#35

Merged
mgautierfr merged 2 commits into
mgautierfr:mainfrom
rursprung:add-check-and-clippy-to-CI
Jun 3, 2025
Merged

CI: add check+clippy+audit#35
mgautierfr merged 2 commits into
mgautierfr:mainfrom
rursprung:add-check-and-clippy-to-CI

Conversation

@rursprung
Copy link
Copy Markdown
Contributor

see the individual commit messages for further details.

@mgautierfr: i noticed that so far your PR settings do not seem to enforce that the CI actually passes. since you're using a build matrix it's cumbersome to do this manually (you'd have to list all combinations of the matrix and update it every time you change that). i've started using the build-results job which i've added here in most of my rust repos nowadays and it works fine. once the PR is merged you can just list this one job as being required for PRs to be merged and then you're set.

@mgautierfr
Copy link
Copy Markdown
Owner

It seems clippy is not installed on nightly in the CI.
But I think it would be better to run clippy only on stable (or make it not mandatory). Nightly clippy may raise new warning and I've found it is pretty annoying to have a CI failing for something not yet published.

@mgautierfr
Copy link
Copy Markdown
Owner

since you're using a build matrix it's cumbersome to do this manually (you'd have to list all combinations of the matrix and update it every time you change that). i've started using the build-results job which i've added here in most of my rust repos nowadays and it works fine. once the PR is merged you can just list this one job as being required for PRs to be merged and then you're set.

Ho nice idea, indeed. enforce different jobs to pass in the github UI is ... not optimal 😆

rursprung added 2 commits June 2, 2025 22:50
check and clippy ensure that the code is warning-free, audit ensures
that the dependencies are vulnerability free.
by adding a single `build-results` job which depends on all other jobs
we can simplify the setting of required builds in the repository.
currently, all builds - including all variations of the build matrix! -
need to be manually specified.

once this has been merged the settings can be changed to require only
this one job (which will fail if any of the other jobs failed).

this way it's also easier to add/remove jobs or change the build matrix
as it no longer requires changing the settings on the repository.

this is inspired by [this discussion on GH][discussion].

[discussion]: https://github.com/orgs/community/discussions/26822
@rursprung rursprung force-pushed the add-check-and-clippy-to-CI branch from 29a4274 to 6c18be7 Compare June 2, 2025 20:50
@rursprung
Copy link
Copy Markdown
Contributor Author

It seems clippy is not installed on nightly in the CI. But I think it would be better to run clippy only on stable (or make it not mandatory). Nightly clippy may raise new warning and I've found it is pretty annoying to have a CI failing for something not yet published.

good point - i've now changed it to only run for stable (and added it to the installation step just in case so that it's always present).

@mgautierfr
Copy link
Copy Markdown
Owner

Looks good to me now !
Thanks for the PR

@mgautierfr mgautierfr merged commit 2e0c5b3 into mgautierfr:main Jun 3, 2025
10 checks passed
@rursprung rursprung deleted the add-check-and-clippy-to-CI branch June 3, 2025 05:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants