GitHub draft pull request feature

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.

1 Like

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.

2 Likes

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.

2 Likes

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

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”.

3 Likes

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?

1 Like

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

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:

3 Likes

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

1 Like

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”.

3 Likes

As an update, Draft PR is working now in CPython.
I have just noticed it today, not sure if it’s been working for sometime :woman_shrugging:t2:

I actually just used it today to let the integration tests run on a PR before setting it as Ready for Review (draftopen). Performing the usual local tests and patchcheck is often enough, but it’s useful to let the final integrations tests run before sending out the notifications to the core devs. Could we potentially add opening the PR as a draft as a “best practices” step before the final submission as way to spread the awareness? If enough contributors are aware of this, it may help to reduce some of the total PR volume so the core devs can manage their time more effectively.

As a non-core, I’ve gone into open PRs where the tests aren’t succeeding a few times to help give recommendations to the newer contributors (such as using patchcheck to fix whitespace issues), but I could still do this for drafts as well, especially if they ask a question in the comments. Preferably the active contributors and triagers could help with these issues so the core devs can spend more time on the more complicated/naunced ones.

Maybe something like that could be included in the devguide?

Yep, that’s exactly what I had in mind! I’d say it would go well under the “Lifecycle of a Pull Request” topic. I can create an issue for it, was just waiting to see what the others thought about it first.

I know this is old topic, but a little over a month ago GitHub added a way to
“convert” a PR to draft, so it’s now a lot more useful:

Two things that are worth keeping in mind are that unlike with labels, converting to draft requires write access to the repository and also, as far as I know, user with write access doesn’t prevent PR author from converting it back to regular PR.

From other things that have been mentioned here, drafts PRs have a different icon from open and closed PRs on the list and there is an is:draft filter (which can also be negated with -) if you want to only see (non-)draft PRs.