GitHub draft pull request feature


(Pablo Galindo Salgado) #1

This is going to be introduced in GitHub:

I wonder if once this is available we can drop the ‘DO-NOT-MERGE’ labels or “WIP” titles and use more extensively to discuss design or similar matters in a better way.


#2

We were just talking about this in bedevere :slightly_smiling_face: (h/t @matrixise) https://github.com/python/bedevere/issues/130

With this feature I think we should utilize it, and remind people to use it instead of wip/do not merge.

On one hand the wip label is nice because we can see it from the Pull Requests tab without needing to go in to the PR. I wonder if there will be similar indicator from within the search result/pull requests tab.


(Yury Selivanov) #3

I wonder if once this is available we can drop the ‘DO-NOT-MERGE’ labels

Please don’t. I use this label to mark PRs as aren’t ready to be merged as a reviewer of the PR, not as an author.


#4

Draft PR also prevents you as maintainer to merge it. Wouldn’t it serve the same purpose as adding “do-not-merge” label?


(Yury Selivanov) #5

It looks like you can create a “draft PR”, but I as a reviewer cannot make somebody else’s PR a “draft PR”. So it’s not as flexible as the “do-not-merge” label, where I can block from merging a PR that isn’t a “draft”.


#6

Gotcha, I see what you’re saying.
Looking at APIs, it doesn’t seem like there is a way to edit the PR into draft mode, so we can’t even build the bot.

But a label is just a label. Wouldn’t it be good to “request changes” with the message “do not merge” instead of adding “do-not-merge” to indicate that this is not complete and you won’t merge it unless they make changes?


#7

Update: available in v4 API (graphql) but not v3 (REST API). Our existing bots are using v3.


(Yury Selivanov) #8

But a label is just a label. Wouldn’t it be good to “request changes” with the message “do not merge” instead of adding “do-not-merge” to indicate that this is not complete and you won’t merge it unless they make changes?

My use case: I approved a few asyncio PRs today, but I don’t want them to be merged until Andrew reviews them as well. Just posting a message might not be enough – some core dev might see a PR that’s approved and hit the “merge” button. If there’s a red “do not merge” label, they would at least scan the messages or ask a question.

This label is a fairly simple mechanism, so unless it’s confusing for some people or breaks the workflow I’d prefer to keep it as is. Or maybe we can come up with a better way of addressing my use case than this label, it’s just that “draft PRs” doesn’t address it either :frowning:


(Jules Lasne (Jlasne)) #9

Have you folks tried making a draft from a fork ? It doesnt seem to work

See:


and

Is it maybe a feature that needs to be enabled in the repository settings ?
EDIT: Doesn’t look like it’s a setting, or at least I couldn’t find it
EDIT2: guys -> folks because I’m an idiot


(Pablo Galindo Salgado) #10

As a friendly side note, there are people that do not identify with the term “guys”, so I would recommend to use more inclusive terms like “folks”.