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

CI: Do not allow merge if labelled DO-NOT-MERGE #103337

Merged
merged 2 commits into from
Apr 9, 2023

Conversation

hugovk
Copy link
Member

@hugovk hugovk commented Apr 7, 2023

For python/core-workflow#498.

Before switching to GitHub's own auto-merge, we need to make sure it respects the DO-NOT-MERGE label.

We can use https://github.com/mheap/github-action-required-labels to prevent merge if a label exists. We will set this new check as required, and it only passes when DO-NOT-MERGE is absent.

Docs: https://github.com/mheap/github-action-required-labels#prevent-merging-if-a-label-exists

This will also disable the merge button for PRs with DO-NOT-MERGE , regardless of using GitHub auto-merge, which will also help avoid accidental merges.

TODO

Demo

In my fork, I've set the check to required and created a demo PR: hugovk#47

No label: can merge

image

With label: cannot merge

image

.github/workflows/require-pr-label.yml Outdated Show resolved Hide resolved
Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

Looks good!
Require PR label seems like an odd name for this workflow, though.

Maybe it can be more generic like Check PR labels?

.github/workflows/require-pr-label.yml Outdated Show resolved Hide resolved
Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

Choose a reason for hiding this comment

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

Great idea overall; I'd been wanting to add something like this for a while now. We should probably backport this at least to 3.11, since automerge functionality is particularly common on auto-backport PRs.

.github/workflows/require-pr-label.yml Outdated Show resolved Hide resolved
.github/workflows/require-pr-label.yml Show resolved Hide resolved
.github/workflows/require-pr-label.yml Outdated Show resolved Hide resolved
@hugovk hugovk added the needs backport to 3.11 only security fixes label Apr 8, 2023
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Shout out to @mheap for developing this useful GH action 👋

Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

Choose a reason for hiding this comment

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

LGTM now, thanks @hugovk !

@hugovk hugovk merged commit 090e26e into python:main Apr 9, 2023
@miss-islington
Copy link
Contributor

Thanks @hugovk for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

@hugovk hugovk deleted the do-not-merge-if-do-not-merge-label branch April 9, 2023 06:39
@bedevere-bot
Copy link

GH-103393 is a backport of this pull request to the 3.11 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Apr 9, 2023
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Apr 9, 2023
(cherry picked from commit 090e26e)

Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
miss-islington added a commit that referenced this pull request Apr 9, 2023
(cherry picked from commit 090e26e)

Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
warsaw pushed a commit to warsaw/cpython that referenced this pull request Apr 11, 2023
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
@bedevere-bot
Copy link

GH-103641 is a backport of this pull request to the 3.10 branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants