Repository navigation
MNT: Remove "Work in progress" and "Experimental" labels in favor of Draft PR? #10706
Description
We have Work in progress and Experimental labels created before GitHub had "Draft PR" feature. Draft PR is nicer because it actually disables the merge button. Should we remove these labels in favor of using Draft PR exclusively?
Might also want to add bot logic to check if a PR is draft or not, so cc @astrofrog , @bsipocz , and @Cadair .
Activity
bsipocz commentedon Sep 2, 2020
maybe the WIP, but experimental is more than a draft and was used in the past to explore alternative approaches and implementation. Also, does all of CI run on draft PRs?
pllim commentedon Sep 2, 2020
Re: CI -- As far as I can see, yes.
mhvk commentedon Sep 3, 2020
Definitely remove "WIP". I'd tend to remove "experimental" also, though agree that it serves a slightly different purpose. But it would be nice to reduce the number of labels...
taldcroft commentedon Sep 13, 2020
I agree that WIP is basically the same as draft, meaning that you have an expectation that this will eventually be mergeable, but just not yet. Experimental is something that is more likely to not get merged.
The bit about having a switch to disable CI seems important. Related is having a way as a maintainer to turn off of CI for a newcomer's half-formed PR that doesn't need to be burning CI for every commit?
One advantage of labels is that they are searchable. Is there a way to find all draft PRs?
bsipocz commentedon Sep 13, 2020
use
draft:truein the searchbox to get the drafts:https://github.com/astropy/astropy/pulls?q=is%3Apr+is%3Aopen+draft%3Atrue
pllim commentedon Sep 14, 2020
CI can be skipped with
[ci skip]or[skip ci], or in our case,[skip travis]as well. We don't have labels that control CI (we control some aspect of the bot with them but that is a different story). So, deleting WIP label and using Draft PR will not change any existing CI behavior.There is currently no way to simply "turn it off for the PR." We can go in and cancel the build or advise author to be diligent in adding
[ci skip]directive in each relevant commit message. I am not sure if we want to simply "turn it off" either, as the decision is subjective and might lead to perception of favoritism.mhvk commentedon Sep 14, 2020
So far, it seems "Draft PR" covers both our "Work in Progress" and "Experimental" even though the two have slightly different meaning, so on the topic of this issue, it would seem we can remove (retire?) the labels.
Separately, if it were possible, I think there would be some advantage to "Draft PR" not running CI by default - often, a draft PR (whether experimental or work in progress) is to show others what one is thinking and an occasional CI run (maybe by temporarily making it a real PR) would be good enough.
pllim commentedon Sep 14, 2020
I think CI control is out of scope of this issue. Currently, both "experimental" and WIP labels do not affect the CI behaviors in any way that I know of.
mhvk commentedon Sep 14, 2020
OK, let's move discussion of CI to #10729
pllim commentedon Sep 16, 2020
Back to the labels, I think there is a consensus to keep "Experimental" and remove "Work in progress"?
Before that label can be removed though, is it acceptable for me to turn all the PRs currently with that label into Draft PR (if not already) without consent from the authors? It seems like I am able to do that but I don't want no drama. https://github.com/astropy/astropy/labels/Work%20in%20progress
mhvk commentedon Sep 16, 2020
Yes, acceptable, I think (and quite a few are mine!)
pllim commentedon Sep 16, 2020
Label is gone. Now you can find WIP PRs as https://github.com/astropy/astropy/pulls?q=is%3Apr+is%3Aopen+is%3Adraft
p.s. I might have forgotten to leave a friendly comment in some of them. Ops.