-
-
Notifications
You must be signed in to change notification settings - Fork 18.3k
workflows/pr: preparation for required status checks #417957
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
69ab2f4
9422f30
09ddb1a
9927d75
caf4ced
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,6 +18,27 @@ concurrency: | |
| permissions: {} | ||
|
|
||
| jobs: | ||
| prepare: | ||
| runs-on: ubuntu-24.04-arm | ||
| outputs: | ||
| mergedSha: ${{ steps.get-merge-commit.outputs.mergedSha }} | ||
| targetSha: ${{ steps.get-merge-commit.outputs.targetSha }} | ||
| systems: ${{ steps.systems.outputs.systems }} | ||
| steps: | ||
| - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 | ||
| with: | ||
| sparse-checkout: | | ||
| .github/actions | ||
| ci/supportedSystems.json | ||
| - name: Check if the PR can be merged and get the test merge commit | ||
| uses: ./.github/actions/get-merge-commit | ||
| id: get-merge-commit | ||
|
|
||
| - name: Load supported systems | ||
| id: systems | ||
| run: | | ||
| echo "systems=$(jq -c <ci/supportedSystems.json)" >> "$GITHUB_OUTPUT" | ||
|
|
||
| check: | ||
| name: Check | ||
| uses: ./.github/workflows/check.yml | ||
|
|
@@ -27,21 +48,67 @@ jobs: | |
|
|
||
| lint: | ||
| name: Lint | ||
| needs: [prepare] | ||
wolfgangwalther marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| uses: ./.github/workflows/lint.yml | ||
| with: | ||
| mergedSha: ${{ needs.prepare.outputs.mergedSha }} | ||
| targetSha: ${{ needs.prepare.outputs.targetSha }} | ||
|
|
||
| eval: | ||
| name: Eval | ||
| needs: [prepare] | ||
| uses: ./.github/workflows/eval.yml | ||
| permissions: | ||
| # compare | ||
| statuses: write | ||
| secrets: | ||
| OWNER_APP_PRIVATE_KEY: ${{ secrets.OWNER_APP_PRIVATE_KEY }} | ||
| with: | ||
| mergedSha: ${{ needs.prepare.outputs.mergedSha }} | ||
| targetSha: ${{ needs.prepare.outputs.targetSha }} | ||
| systems: ${{ needs.prepare.outputs.systems }} | ||
|
|
||
| labels: | ||
| name: Labels | ||
| needs: [eval] | ||
| uses: ./.github/workflows/labels.yml | ||
| permissions: | ||
| issues: write | ||
| pull-requests: write | ||
| statuses: write | ||
|
|
||
| reviewers: | ||
| name: Reviewers | ||
| needs: [prepare, eval] | ||
| if: needs.prepare.outputs.targetSha | ||
| uses: ./.github/workflows/reviewers.yml | ||
| secrets: | ||
| OWNER_APP_PRIVATE_KEY: ${{ secrets.OWNER_APP_PRIVATE_KEY }} | ||
|
|
||
| build: | ||
| name: Build | ||
| needs: [prepare] | ||
| uses: ./.github/workflows/build.yml | ||
| secrets: | ||
| CACHIX_AUTH_TOKEN: ${{ secrets.CACHIX_AUTH_TOKEN }} | ||
| with: | ||
| mergedSha: ${{ needs.prepare.outputs.mergedSha }} | ||
|
|
||
| # This job's only purpose is to serve as a target for the "Required Status Checks" branch ruleset. | ||
| # It "needs" all the jobs that should block merging a PR. | ||
| # If they pass, it is skipped — which counts as "success" for purposes of the branch ruleset. | ||
| # However, if any of them fail, this job will also fail — thus blocking the branch ruleset. | ||
| no-pr-failures: | ||
| # Modify this list to add or remove jobs from required status checks. | ||
| needs: | ||
| - check | ||
| - lint | ||
| - eval | ||
| - build | ||
| # WARNING: | ||
| # Do NOT change the name of this job, otherwise the rule will not catch it anymore. | ||
| # This would prevent all PRs from merging. | ||
| name: no PR failures | ||
| if: ${{ failure() }} | ||
| runs-on: ubuntu-24.04-arm | ||
| steps: | ||
| - run: exit 1 | ||
|
Comment on lines
+96
to
+114
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I assume, before merging, we should confirm in NixOS/org#130 that we are definitely planning to enable required status checks at some point? I can't see why we wouldn't want to... But if the proposal was blocked for any reason, then we wouldn't need to merge this job. Alternatively, we could merge it optimistically so that we can see it working in production. Once we've proven the CI is working, it will be easier to argue in favour of requiring it. With this approach, we can still remove the job later if the proposal is blocked or we change our minds (for whatever reason).
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I'd say let's do that, yes. It's not going to do any harm if unused - especially with the new naming that doesn't say anything about "required" anymore. |
||
Uh oh!
There was an error while loading. Please reload this page.