Skip to content
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

continuous-integration.yml: use one job to collect the status of all … #4316

Merged
merged 1 commit into from
Dec 25, 2023

Conversation

BacLuc
Copy link
Contributor

@BacLuc BacLuc commented Dec 24, 2023

…jobs in the workflow

That we can use one required status check in the branch protection rules. This allows to:

  • Allows review of the settings which jobs should be required. The merge of a pr leads to the new settings being active
  • Not having to synchronize the settings of the required status check with the devel branch. This happens automatically because the configuration is checked in.
  • Not having to merge/rebase devel into PRs because the names of the jobs changed but the old/new job names are in the required status checks.
    (see Contribution documentation update #4204 (comment))

@carlobeltrame
Copy link
Member

This also makes it possible to break the CI on devel whenever we change the required status checks, right? But we can fix it when it happens.

…jobs in the workflow

That we can use one required status check in the branch protection rules.
This allows to:
- Allows review of the settings which jobs should be required. The merge of a pr leads to the new settings being active
- Not having to synchronize the settings of the required status check with the devel branch.
This happens automatically because the configuration is checked in.
- Not having to merge/rebase devel because the names of the jobs changed but the old/new job
names are in the required status checks.
(see ecamp#4204 (comment))
@BacLuc
Copy link
Contributor Author

BacLuc commented Dec 24, 2023

This also makes it possible to break the CI on devel whenever we change the required status checks, right? But we can fix it when it happens.

My change improves this, because we don't have to change the branch protection rules anymore.
As long as the required checks pass, the ci passes in devel too

@carlobeltrame
Copy link
Member

This also makes it possible to break the CI on devel whenever we change the required status checks, right? But we can fix it when it happens.

My change improves this, because we don't have to change the branch protection rules anymore.
As long as the required checks pass, the ci passes in devel too

I don't think so. Assume we add a new required status check to the workflow on devel. Now any old PRs can fail this newly required status check, but still be merged. But again, this should be rare enough to fix as we go.

@BacLuc
Copy link
Contributor Author

BacLuc commented Dec 24, 2023

Let me do it.
I already did this in another Project and IT worked Like a charm

@usu usu added this pull request to the merge queue Dec 25, 2023
Merged via the queue into ecamp:devel with commit 2f9efcc Dec 25, 2023
32 checks passed
@BacLuc BacLuc deleted the ci-one-required-check branch December 29, 2023 22:44
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.

None yet

3 participants