Skip to content

Comments

Enforce PR failure based on a single job and fail the job if a dependent job was skipped.#392

Merged
adiroiban merged 3 commits intomasterfrom
ci-enforce-all-success
Jun 28, 2022
Merged

Enforce PR failure based on a single job and fail the job if a dependent job was skipped.#392
adiroiban merged 3 commits intomasterfrom
ci-enforce-all-success

Conversation

@adiroiban
Copy link
Member

@adiroiban adiroiban commented Jun 27, 2022

This tries to work around an issue with GitHub.

We have a PR configured to allowing merging only if Job A is green.

If you have a job A depending on another job B, if job B fails, job A is skipped.

But with job A skipped, GitHub is happy and it allows the PR to be merged.

Implement the nice move from @altendky from Chia-Network/chia-blockchain#11880

@codecov
Copy link

codecov bot commented Jun 27, 2022 •
edited
Loading

Codecov Report

Merging #392 (1ae8e20) into master (2cf5473) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #392   +/-   ##
=======================================
  Coverage   97.60%   97.60%           
=======================================
  Files          22       22           
  Lines        1334     1334           
  Branches      130      130           
=======================================
  Hits         1302     1302           
  Misses         16       16           
  Partials       16       16

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2cf5473...1ae8e20. Read the comment docs.

@adiroiban
Copy link
Member Author

adiroiban commented Jun 27, 2022

This PR is started without a news fragment to test that the catch-all job fails.

@adiroiban adiroiban requested a review from altendky June 27, 2022 15:08
@adiroiban
Copy link
Member Author

adiroiban commented Jun 27, 2022

@altendky @graingert do you have time to take a look and see if we can merge this ?

Thanks :)

@adiroiban adiroiban requested a review from graingert June 27, 2022 15:09
altendky
altendky approved these changes Jun 27, 2022
View reviewed changes
Copy link
Member

@altendky altendky left a comment

Choose a reason for hiding this comment

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

It reads as I would expect. I look forward to hearing nothing more about this because it works. :] *fingers crossed* Thanks.

@adiroiban adiroiban merged commit 57a72eb into master Jun 28, 2022
@adiroiban adiroiban deleted the ci-enforce-all-success branch June 28, 2022 00:17
@adiroiban adiroiban mentioned this pull request Jun 28, 2022
Closed
@adiroiban adiroiban mentioned this pull request Jul 8, 2022
Merged
@webknjaz
Copy link
Contributor

webknjaz commented Aug 23, 2022

Hey, I noticed you're reinventing the wheel here :)
https://github.com/re-actors/alls-green

@webknjaz webknjaz mentioned this pull request Aug 23, 2022
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@altendky altendky altendky approved these changes

@graingert graingert Awaiting requested review from graingert

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@adiroiban @webknjaz @altendky