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

MNT: Remove "Work in progress" and "Experimental" labels in favor of Draft PR? #10706

Closed
pllim opened this issue Sep 2, 2020 · 12 comments
Closed

Comments

@pllim
Copy link
Member

pllim commented Sep 2, 2020

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 .

@bsipocz
Copy link
Member

bsipocz commented 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
Copy link
Member Author

pllim commented Sep 2, 2020

Re: CI -- As far as I can see, yes.

@mhvk
Copy link
Contributor

mhvk commented 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
Copy link
Member

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
Copy link
Member

bsipocz commented Sep 13, 2020

use draft:true in the searchbox to get the drafts:

https://github.com/astropy/astropy/pulls?q=is%3Apr+is%3Aopen+draft%3Atrue

@pllim
Copy link
Member Author

pllim commented Sep 14, 2020

The bit about having a switch to disable CI seems important

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.

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?

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
Copy link
Contributor

mhvk commented 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
Copy link
Member Author

pllim commented 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
Copy link
Contributor

mhvk commented Sep 14, 2020

OK, let's move discussion of CI to #10729

@pllim
Copy link
Member Author

pllim commented 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
Copy link
Contributor

mhvk commented Sep 16, 2020

Yes, acceptable, I think (and quite a few are mine!)

@pllim
Copy link
Member Author

pllim commented 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.

@pllim pllim closed this as completed Sep 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants