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

Introduce a centralized GHA check/gate job #543

Merged
merged 1 commit into from
Dec 2, 2022

Conversation

webknjaz
Copy link
Member

@webknjaz webknjaz commented Dec 1, 2022

This patch is intended to lower the maintenance burden of having to manually go through each matrix subjob name in the branch protection repository settings. It allows to only include the check job in the branch protection and it will robustly determine if the dependencies have succeeded or not.

It is currently mostly serves the Python ecosystem in projects like aiohttp, attrs, cryptography, pydantic, open edX, pip etc. But I've also seen other communities picking it up lately, like the AWS Rust SDK and even the engine powering https://dev.to, to my surprise. Strictly speaking, it is agnostic.

Ref: https://github.com/marketplace/actions/alls-green#why

.github/workflows/test.yml Outdated Show resolved Hide resolved
.github/workflows/test.yml Outdated Show resolved Hide resolved
@webknjaz webknjaz requested review from layday and henryiii and removed request for layday and henryiii December 1, 2022 20:49
This patch is intended to lower the maintenance burden of having to
manually go through each matrix subjob name in the branch protection
repository settings. It allows to only include the `check` job in the
branch protection and it will robustly determine if the dependencies
have succeeded or not.

It is currently mostly serves the Python ecosystem in projects like
aiohttp, attrs, cryptography, pydantic, open edX, pip etc. But I've
also seen other communities picking it up lately, like the AWS Rust
SDK and even the engine powering https://dev.to, to my surprise.
Strictly speaking, it is agnostic.

Ref: https://github.com/marketplace/actions/alls-green#why
@webknjaz webknjaz requested review from henryiii and removed request for layday December 1, 2022 22:20
@webknjaz webknjaz requested a review from layday December 2, 2022 11:43
Copy link
Member

@layday layday left a comment

Choose a reason for hiding this comment

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

Thanks!

@webknjaz
Copy link
Member Author

webknjaz commented Dec 2, 2022

@layday anything else needed for the merge? Note that there's an action item to swap out the branch protection settings that is post-merge. Don't do this before merging, since it'll block other PRs until something is able to report a check with this name.

@layday
Copy link
Member

layday commented Dec 2, 2022

I don't have permission to change the branch protection settings, so I'll leave this to @henryiii or @FFY00 to merge.

@webknjaz
Copy link
Member Author

webknjaz commented Dec 2, 2022

@layday FWIW merging this is harmless, it's the branch protection bit that requires extra privileges. But merging will just add an extra check reported but not used, which is fine. I'd even recommend doing so earlier than later because all the PRs with the old workflow definitions will have to be restarted to report this check. The earlier this happens, the less friction switching the branch protection setting will cause.

@webknjaz
Copy link
Member Author

webknjaz commented Dec 2, 2022

cc @henryiii

@henryiii henryiii merged commit b8a1384 into pypa:main Dec 2, 2022
@henryiii
Copy link
Contributor

henryiii commented Dec 2, 2022

Thanks!

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