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 gate/check GHA job #10315

Merged
merged 2 commits into from
Jun 18, 2024

Conversation

webknjaz
Copy link
Member

@webknjaz webknjaz commented Sep 24, 2022

This adds a GHA job that reliably determines if all the required
dependencies have succeeded or not.

It also allows to reduce the list of required branch protection CI
statuses to just one — check. This reduces the maintenance burden
by a lot and have been battle-tested across a small bunch of projects
in its action form and in-house implementations of other people.

I was curious about the spread of use. And I've just discovered that
it is now in use in aiohttp (and other aio-libs projects), CherryPy,
some of the Ansible repositories, all of the jaraco's projects (like
setuptools, importlib_metadata), some PyCQA, PyCA and pytest
projects, a few AWS Labs projects. Admittedly, I maintain a few of
these but it seems to address some of the pain folks have:
jaraco/skeleton#55 (comment).

The story behind this is explained in more detail at
https://github.com/marketplace/actions/alls-green#why.

@@ -188,3 +188,17 @@ jobs:
fail_ci_if_error: true
files: ./coverage.xml
verbose: true

check: # This job does nothing and is only used for the branch protection
Copy link
Member Author

Choose a reason for hiding this comment

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

^ this would have to go into the branch protection settings

Copy link
Member

@RonnyPfannschmidt RonnyPfannschmidt left a comment

Choose a reason for hiding this comment

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

I like it, let's figure a better I'd for the job and go for it

@@ -188,3 +188,17 @@ jobs:
fail_ci_if_error: true
files: ./coverage.xml
verbose: true

check: # This job does nothing and is only used for the branch protection
Copy link
Member

Choose a reason for hiding this comment

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

Let's give it a better name than check

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, PyCA uses all-green, some use all-successful. I originally was going to recommend gate but a colleague convinced me that check is less confusing (because it doesn't fully map to https://gating.dev and openstack's understanding of gating).
This is why I left check in the action readme example so most people would just copy-paste that. For consistency, I'd have the same job id across the board but maybe specify a differing name: value?

Copy link
Member

Choose a reason for hiding this comment

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

Different name is fine, it's just thst check is absolutely useless in terms of deriving meaning given gh naming schemes

Required-checks-agregate would be far more telling for example

Also there is possibly a need to limit who can change the dependencies

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay, I'll let the maintainers bikeshed on the name, then.
As for updating branch protection in the repo settings, it's usually available for the repo admins, I think.

Copy link
Member

Choose a reason for hiding this comment

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

Ill give it a spin this evening

@webknjaz
Copy link
Member Author

webknjaz commented Oct 4, 2022

@RonnyPfannschmidt you could also bike shed post-merge.

This adds a GHA job that reliably determines if all the required
dependencies have succeeded or not.

It also allows to reduce the list of required branch protection CI
statuses to just one — `check`. This reduces the maintenance burden
by a lot and have been battle-tested across a small bunch of projects
in its action form and in-house implementations of other people.

I was curious about the spread of use. And I've just discovered that
it is now in use in aiohttp (and other aio-libs projects), CherryPy,
some of the Ansible repositories, all of the jaraco's projects (like
`setuptools`, `importlib_metadata`), some PyCQA, PyCA and pytest
projects, a few AWS Labs projects. Admittedly, I maintain a few of
these but it seems to address some of the pain folks have:
jaraco/skeleton#55 (comment).

The story behind this is explained in more detail at
https://github.com/marketplace/actions/alls-green#why.
@webknjaz
Copy link
Member Author

webknjaz commented Dec 7, 2023

@RonnyPfannschmidt looks like this fell off your radar?

@RonnyPfannschmidt RonnyPfannschmidt merged commit dc65bb6 into pytest-dev:main Jun 18, 2024
25 checks passed
Copy link

patchback bot commented Jun 19, 2024

Backport to 8.2.x: 💚 backport PR created

✅ Backport PR branch: patchback/backports/8.2.x/dc65bb6a6699bdeb5ee7523245db1ee724670e29/pr-10315

Backported as #12484

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Jun 19, 2024
Introduce a gate/check GHA job

(cherry picked from commit dc65bb6)
RonnyPfannschmidt added a commit that referenced this pull request Jun 19, 2024
…c65bb6a6699bdeb5ee7523245db1ee724670e29/pr-10315

[PR #10315/dc65bb6a backport][8.2.x] Introduce a gate/check GHA job
@webknjaz webknjaz added the type: infrastructure improvement to development/releases/CI structure label Jun 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: infrastructure improvement to development/releases/CI structure
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants