
Introducing draft pull requests - nickvanw
https://github.blog/2019-02-14-introducing-draft-pull-requests/
======
xrd
When I was interviewing GitHub employees for my book
([https://buildingtoolswithgithub.teddyhyde.io/](https://buildingtoolswithgithub.teddyhyde.io/))
one of them made a surprising comment. Early on, his GitHub teammates told him
he should create an empty PR with a plan of attack before writing any code. I
thought it was a brilliant idea and most teams don't do this. Smart of them to
make this a separate feature instead of just bolting on WIP.

~~~
DogLover_
Someone suggested this on my team. I personally don’t like the idea because
these policies often times lead to bureaucracy and then nothing getting
released. It is not that I am against thinking ahead but if i have to in
details explain everything I do, then more time is spent documenting than
actually creating which is the part I enjoy.

~~~
xrd
I get it, that's indeed the fun part.

But, do you believe the statistic that 90% of the cost of software is the
maintenance?

If so, then the only way you can be a mythical 10x engineer is if you are
willing to do things that really reduce the team costs. Everything else about
10x engineers is just mythology.

I think this is an example / practice of one of those things you can do for
your team. Communicating clearly up front, being willing to discuss and alter
poor thinking (everyone does it) is what turns individual contributors into
powerful team members. If you just start with coding, you are missing all that
part of it.

~~~
DogLover_
Documenting/communicating what you do is important but when it becomes a
process it can really be detrimental. Every new feature won’t take the same
amount of time and having to spent time writing down my approach for a task
that will only take a few days will likely just limit me. That time could be
spent on writing (more) tests which is also a form of documentation. If we say
that 9/10 times a solution is good, the time saved not writing down the
approach could accumulate to 1 rewrite. Basically for me it boils down to some
of the agile values and principless:

\- Individuals and interactions over processes and tools

\- Working software over comprehensive documentation

\- Working software is the primary measure of progress.

\- The most efficient and effective method of conveying information to and
within a development team is face-to-face conversation.

[https://agilemanifesto.org/principles.html](https://agilemanifesto.org/principles.html)

------
jedberg
Help me understand something -- right now my workflow is to create a new
branch when I'm working on a new feature. One branch per feature. I check in
frequently and push up so my work is saved.

Sometimes I ask people to look at the code in progress when I want opinions on
architecture or just overall code quality.

When I think the feature is ready for merging, I'll create a pull request.

How does a draft PR help me? What do I get with a draft PR that I don't
already have?

I assume it's a good idea, and everyone here seems to think so too, so I think
I'm missing something obvious.

Edit: Lots of great points below, mostly pointing out features that I don't
really use a lot, which is why I didn't see the improvement. Thanks everyone!

~~~
mattmcknight
I see PRs with titles prefixed "DO NOT MERGE" or "WIP". People want the UI
that goes along with a PR, prior to it being ready to merge.

~~~
cco
You don't have a review mandate before merge?

~~~
mattmcknight
Yeah, but people might approve it for merge when it is not obvious that it
isn't ready.

~~~
cco
Wouldn't this same person do the same for a Draft PR? i.e. if they're just
reviewing things wouldn't they just set the Draft PR 'ready to review' and
then review it?

Or maybe I'm confused by your answer, are people commonly reviewing PR's
without being asked?

~~~
CraftThatBlock
What the OP is saying is that someone can review a PR even when it's not ready
by accident (not seeing WIP in title/tag), while with a draft, the reviews are
disabled until its ready for review.

This basically is a better WIP system than before because it can prevent
accidental reviews and wasting time of re-reviewing when it actually is ready.

------
rhinoceraptor
It would also be nice to have a way to link multiple PRs together so they can
be merged simultaneously. A ton of people are working across multiple repos
which makes reviewing and coordinating changes really annoying.

~~~
morley
Why not have everyone create their PRs against a feature branch instead of
master, and merge the feature branch into master when the project is done?

~~~
rhinoceraptor
I don't mean multiple PRs for the same repo, I mean multiple PRs across
multiple repos.

------
markovbot
I'm assuming this is a response to GitLab's concept of a WIP merge request.

~~~
dkhenry
I don't know if I would call it GitLab's concept. Every place I have worked at
that has used github has had the idea of a Work In Progress pull request. Its
a pretty natural concept for an multi-member development team. So much so that
all of the places I have used it have written explicit CI checks that would
check for a label on the PR or the title to be in a given format and would
fail to make sure you couldn't merge the branch until WIP was removed.

~~~
raphlinus
Gerrit has had the concept of "draft" for a long time. It's also better at
multiple "patchsets" as a "changelist" evolves, some of which can have
rebasing without needing a forced update. On the other hand, it's somewhat
hacky the way it layers on top of git, requiring a "change id" to link the
different patchsets together. Much of the way it works came from trying to
deliver a similar experience as Perforce.

Disclosure: worked at Google, have used both systems a lot.

~~~
hipnoizz
I found a small inconvenience of maintaining ChangeId (usually handled
automatically by the git hook) a very small price to pay for all great
features provided by Gerrit. I know that Gerrit team investigated few times a
better way to attach this metadata to commits but so far it works this way.

What other 'hacky ways' you mean? 'Magic' refs/for/* branches can be a bit
weird at beginning. On the other hand introducing Gerrit 7-8 years ago to my
back-then employer made me understand the Git model much better. Anyway, e.g.
IntelliJ IDEA plugin for Gerrit should alleviate almost all pain points (if
someone stick to JetBrains products).

Were you working on Android/AOSP? I always assumed that this is the primary
project where Googlers have a chance to work with Gerrit. I thought that
Google uses some in-house CR system, as mentioned by
[https://www.gerritcodereview.com/about.html](https://www.gerritcodereview.com/about.html).

~~~
raphlinus
Yes, Android then Fuchsia. I agree it's good, overall I prefer it to Github,
but by "hacky" I mostly mean it's very much out of step from the way the rest
of the ecosystem works. This is much more true for repo / jiri (which is
Google's own spin on the submodule problem), but includes change ids.

------
jillesvangurp
Nice, I tend to like having pull requests created early so people can follow
what is happening and give early feedback.

~~~
wooly_bully
Yeah, my normal Gitlab workflow is:

1\. Start a branch with a first commit 2\. Open a PR and write details /
background 3\. Mark WIP 4\. Eventually remove WIP and add someone as reviewer.

Glad to see this is available now.

~~~
systemtest
In my team we use the pipeline to automatically create the merge request after
the branch is pushed.

------
peterwwillis
This blog post really doesn't sell this feature well. How is this any
different at all from me just putting "WIP" in the title of the PR? Ok, so it
prevents merging a WIP PR - who would do that?

~~~
bswinnerton
If you use the `CODEOWNERS` feature, it won't auto-ping anyone until it comes
out of the draft state.

~~~
peterwwillis
And that's useful (if you use CODEOWNERS) - but just having a checkbox that
says "don't ping the CODEOWNERS" on all PRs would have the same effect without
needing a different type of PR.

~~~
michaelmior
Aren't "PRs which should ping the CODEOWNERS" and "PRs which shouldn't" two
types of PR anyway? Personally, I think it's useful calling it a draft and
preventing the merge anyway.

------
house9-2
I just use a WIP label on the PR, keep it simple.

~~~
felixfbecker
That will send notifications to all code owners, which a draft PR will not

------
dpeck
Love it. I’ve had success using Issues in the past to do get implementation
feedback, and have a “one pager” for whatever I/the team was working on at the
time, but it didn’t feel like it stuck very well with junior team members.

Having something like this that allows a little closer relationship between
the plan and the code created for the plan is nice.

------
fyhn
This is good. We have been emulating this with either a "don't review yet" tag
or by opening a pull request without setting reviewers, but it's clearly
better to have support for it built in. More people will use it, and I don't
have to train people in our particular way of indicating not-ready pull
requests.

------
aidos
Very happy about this. We recently switched to github and I asked my team to
start their PRs immediately. Was pretty disappointed to discover that they
needed something to commit first. Having a draft state will in itself be
really useful.

------
creyes
This is rad. I usually like to put up a PR early and add a WIP label. This
lets my teammates take a look at the early direction of whatever I'm doing and
I can make changes as necessary. This is a small but really impactful feature!

------
saagarjha
It’d be nice to have some way to configure exactly how the “draft pull
request” works, because there are a couple cases where I think this could be
improved:

* Notifying the maintainers might be useful in some cases. Often, I will file a WIP pull request to get preliminary review by a maintainer so that I know I’m going in the right direction.

* Being able to convert a draft pull request into a full one: Useful if the original author has stopped responding and you’d like to merge in their changes (after tweaking them, etc.)

------
gus_massa
> _Also, if you have a CODEOWNERS file in your repository, a draft pull
> request will suppress notifications to those reviewers until it is marked as
> ready for review._

I'm not sure. When I send a PR marked with PR I usually want to know the
opinion of the maintainer/owner to know if it is going in the right direction
and if it worth finishing.

------
yingw787
I love this! I draft pull requests as an ad-hoc branch management scheme at
work with a [WIP] prefix, and reviewers are notified when I make a change
(small team so not too spammy). When it's ready I update the summary and
remove [WIP]. There might be a better way to do it (and I'd love to hear
suggestions), but this is how I do it right now.

------
meowface
Looks great. Any idea when it'll be added to GitHub Enterprise? This would be
really helpful for my team.

------
michaelwda
I literally saw someone ask for this feature on Twitter last week.
VSTS/VSO/DevOps/? already has something similar.

This seems like the kind of feature that probably takes longer than a week to
build and test, so I'm guessing it was already in the works.

------
joeldrapper
I count at least 17,800 requests for this feature.
[https://www.google.com/search?q=%22WIP%22+inurl%3Apull+site%...](https://www.google.com/search?q=%22WIP%22+inurl%3Apull+site%3Agithub.com)

~~~
ken
I count 17,800 cases where someone accomplished the same thing by typing 3
letters.

I've done this many times myself, and I hope nobody was counting those as a
feature request.

------
dvtrn
I can already see this coming in handy on our private team repo as I was not
far off from revoking someone's merge permissions for abusing PRs, this will
be a nice little way to 'jail' the offending party.

------
Scarblac
We have Github Team and I don't see the feature. Still rolling out?

------
reificator
This will be helpful for the cultural clash that can sometimes happen when a
new contributor tries to do something like this and the maintainers expect all
PRs to be immediately merge-worthy.

------
lowii
Why? Aren't all merge requests a WIP until they are merged?

~~~
fyhn
The difference is whether the author of the pull request considers his code
not ready for review or not. Both are work in progress but they need different
treatment.

------
emersion
Is there a way to convert a regular PR to a draft PR?

~~~
domoritz
I need this as well. I have a ton of PRs with WIP labels.

------
KuhlMensch
Hey niiiice, opening PRs early is definitely the way I work. This will
definitely help with PR grooming :)

------
winrid
This is nice. I've always used the WIP tag but this is cleaner since it blocks
merging.

------
royosherove
I could be wrong, but I feel this actually enables the wrong kind of behavior
we should expect from developers. We want fast, continuously integrated code
that is merged early. Usually that means using feature toggles so that we can
see compilation issues without the code being active by default.

This feature signals "yeah it's OK to take your time and not merge stuff
incrementally - here's a feature just for that since it's such an important
part of your workflow". Yes, people have been doing it anyway but at least it
felt a bit like a hack, like it's not supposed to work that way.

Am I crazy, or is this pushing back progress made in CI coding practices?

~~~
chrisseaton
> We want fast, continuously integrated code that is merged early

For some projects maybe.

Other projects want changes to be very carefully presented, considered and
reviewed.

> it's not supposed to work that way

According to who? Is this how you do things at work? Ok, but other people work
differently. There’s not authority in programming to set what we’re ‘supposed’
to do.

> is this pushing back progress made in CI coding practices?

Seems orthogonal to me. You can run CI on a branch.

But overall, no not everyone’s development is a mad dash to get everything
merged instantly and some want carefully created PRs.

~~~
benjiweber
> Seems orthogonal to me. You can run CI on a branch.

Funny how some terms can be diffused enough over time to come to mean almost
the opposite of what they originally meant. Continuous Integration is about
integrating continuously. "the practice of merging all developer working
copies to a shared mainline several times a day." i.e. everyone commits to
master multiple times a day.

Then tooling gets built to facilitate this way of working. People start
confusing build tooling with CI the practice. Eventually that tooling gets
used to support people working in continual isolation on feature branches.
People start calling this practice CI even though there's no integration in
sight.

Of course true CI isn't applicable for all contexts. Draft pull requests look
like a great tool for where CI is impractical.

~~~
chrisseaton
Merging into a single branch isn’t the definition that people like
ThoughtWorks seem to use. They just talk about integrating into a shared
respository, not specifically a shared branch.

The CI system then does the work for you to test that on top of master. You’re
talking about developers doing things that the CI system is there to do.

~~~
gunnarmorling
CI only seems to make sense to me when talking about a single branch
("master", "trunk") into which all changes are frequently merged. FWIW, also
Wikipedia describes it as "merging all developer working copies to a shared
mainline", the latter being a branch, not repo.

Otherwise, you still might test indvidual feature branches on top of master,
but that wouldn't prevent any conflicts between several of the pending
branches. One could think of a system that continuously tests all possible
combinations of all pending feature branches to spot any conflicts before
actually integrating them. Not sure whether such thing exists, though, or
whether it'd be very practical.

