Skip to content

ci: group required jobs, add alls-green#1590

Merged
DavSanchez merged 5 commits intomainfrom
ci/required-jobs
Aug 28, 2025
Merged

ci: group required jobs, add alls-green#1590
DavSanchez merged 5 commits intomainfrom
ci/required-jobs

Conversation

@DavSanchez
Copy link
Contributor

@DavSanchez DavSanchez commented Aug 28, 2025

What this PR does / why we need it

Groups jobs performed for PRs (checks and tests) into a single workflow and adds a final step with alls-green to fail if any of the depending jobs fail. This enables setting only the alls-green job as required in the repo settings, enforcing a certain degree of CI status to merge.

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

  • Provided a meaningful title following conventional commit style.
  • Included a detailed description for the Pull Request.
  • Documentation under docs is aligned with the change.
  • Follows guidelines for Pull Requests in CONTRIBUTING.md.
  • Follows log level guidelines.

@DavSanchez DavSanchez requested a review from a team as a code owner August 28, 2025 00:51
@sigilioso
Copy link
Contributor

The unexpected unit-test failure shows that the all-green thing is working as expected 😄
image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was merging push_pr_checks and push_pr_test too much?

Copy link
Contributor Author

@DavSanchez DavSanchez Aug 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have merged them in fa81509 (and updated PR description)! Now there's only one alls-green to add to the repo settings.

@DavSanchez DavSanchez enabled auto-merge (squash) August 28, 2025 11:19
- name: Check code formatting
run: cargo fmt --check

checks:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename?

- release-build
- unit-docs-onhost-integration
- test-with-root
- embedded
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we rename also this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean only the embedded?

- coverage
- checks
- third-party-notices-check
- fmt
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we call them
cargo-fmt
cargo-doc
cargo-clippy

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have renamed several of the jobs. LMKWYT!

Comment on lines +410 to +411
check-all-green:
name: 🟢 All required checks and tests pass
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do 'Github settings' require the job key or the job name when configuring required jobs? I wonder if it will work well with the emoji 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works fine, I remember when adding the third party notices check since it had an emoji as well:
image

Copy link
Contributor

@sigilioso sigilioso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a question regarding how to set this up. Thanks for taking care! 🚀

@DavSanchez DavSanchez merged commit 82ed217 into main Aug 28, 2025
27 checks passed
@DavSanchez DavSanchez deleted the ci/required-jobs branch August 28, 2025 11:54
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.

3 participants

Comments