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

Disable merge button for certain labels #90

Closed
bsipocz opened this issue Sep 24, 2019 · 18 comments · May be fixed by #91
Closed

Disable merge button for certain labels #90

bsipocz opened this issue Sep 24, 2019 · 18 comments · May be fixed by #91
Labels
enhancement New feature or request

Comments

@bsipocz
Copy link
Member

bsipocz commented Sep 24, 2019

What a cunning plan!

It would be nice to be able to disable the merge button while a PR has a certain label set.

E.g. astropy/astropy#9282 needs to be tested in a downstream package, so ideally it should not be merged until the needs-downstream-testing label is present. I suspect this logic could be used for other labels, too.

@bsipocz bsipocz added the enhancement New feature or request label Sep 24, 2019
@Cadair
Copy link
Member

Cadair commented Sep 24, 2019

by "disable merge button" I assume you mean set a failing check?

@bsipocz
Copy link
Member Author

bsipocz commented Sep 24, 2019

something stronger, to have the greyed out merge button, similarly when no one yet approved it.

@Cadair
Copy link
Member

Cadair commented Sep 24, 2019

Is that something you can do via the github API?

@bsipocz
Copy link
Member Author

bsipocz commented Sep 24, 2019

I'm not sure. Did I mention it was cunning plan? 😏

@Cadair
Copy link
Member

Cadair commented Sep 24, 2019

The way I know will work is that you setup a specific check for the label check, and then make it a required check using protected branches. I can't see anything in the API about blocking merge.

@bsipocz
Copy link
Member Author

bsipocz commented Sep 24, 2019

Oh, that would work quite nicely I suppose.

@jlorenzoC
Copy link

jlorenzoC commented Oct 22, 2021

Unless you wanna define your own action for this, you can use this one: https://github.com/marketplace/actions/enforce-pr-labels

@stefanv
Copy link

stefanv commented Aug 30, 2024

It would be neat to have this. Just landed here after skimage wanted the same, but I believe it would have to be implemented on a GH level.

@bsipocz
Copy link
Member Author

bsipocz commented Aug 30, 2024

I don't think we can grey out the merge button, but there could be a required workflow that fails when the label is present.

I think people with just write level permissions would not be able to override such a branch protection.

@stefanv
Copy link

stefanv commented Aug 30, 2024

This is what we have on scikit-image, but it's not great when newcomers get a red cross on their PR, without the ability to turn it green themselves :/ [In our case, the requirement is to have a type: ... label, for changelog purposes.]

@bsipocz
Copy link
Member Author

bsipocz commented Aug 30, 2024

Ah, the circle closes, I believe I was complaining about that failing labelling job myself 😂

@pllim
Copy link
Contributor

pllim commented Aug 30, 2024

FWIW I think this can be accomplished in Actions now, without baldrick. For example: https://www.neilmacy.co.uk/blog/github-action-to-block-merging + branch protection rules.

@bsipocz
Copy link
Member Author

bsipocz commented Aug 30, 2024

Yes, this is what we chatted about above that we can currently have a failing required workflow/job controlled by a label, but that red cross could be annoying and or confusing to contributors (and also reviewers, I certainly could get less attention from maintainers' on some of my PRs when there were CI failures, even if they were clearly unrelated)

@pllim
Copy link
Contributor

pllim commented Aug 30, 2024

If the red cross only shows up when a maintainer chooses to apply a label, then it would be green most of the time. Personally I will be okay with that.

There is also a totally unrelated request for GitHub to just make all PRs draft by default (merge button is automatically disabled) but after so many years, that request is still open.

@bsipocz
Copy link
Member Author

bsipocz commented Aug 30, 2024

Yeap, original use case for this issues, the downstream testing label, would indeed be rare, but skiimage's case is the opposite, it would not allow merging before adding certain labels.

@pllim
Copy link
Contributor

pllim commented Aug 30, 2024

Hmm... You could also require N number of approvals before merge. I think that disables the merge button until approvals given without any red X ?

@pllim
Copy link
Contributor

pllim commented Aug 30, 2024

Regardless, I feel like this has nothing to do with baldrick anymore. Is there an issue over at sklearn scikit-image we can continue on?

@bsipocz
Copy link
Member Author

bsipocz commented Aug 30, 2024

Yes, agreed on closing this for baldrick. It was a cunning plan that didn't work out 🤣

@bsipocz bsipocz closed this as completed Aug 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants