[owncloud-devel] Pull Requests suggested guidelines

Vincent Petry pvince81 at owncloud.com
Mon Mar 17 10:21:08 MET 2014


Hi,

I wrote a small list of guidelines about Pull Requests which I think
should help improve the reviewing process a bit:

1. From Tanghus' comment on a calendar app PR: "When making a PR always
limit it to a single issue, or it will get stuck for review very likely
until it's too outdated to ever get merged."

2. Summarize what the PR does to make it easier to understand for people
who don't know that part of the code/app. Sometimes one needs to read a
long issue thread before they can understand what it is about, which is
usually discouraging for reviewers/testers.

3. Add test steps to your PRs with checkboxes, it makes it much quicker
to understand what needs to be tested. It also helps you make sure you
covered all cases.

I usually write the test cases and test them myself, ticking the boxes.
Then reviewers can just go through the same steps to make sure it works
on their side.

Also, writing down the steps confirms that you've tested it. (you do
test your changes, do you ? ;-))

These also help when testing backports.

4. When you start working on a big task, open a PR with the prefix
"[WIP]" and a nice :construction: emoji. This helps making other people
aware of your progress and gives them a chance to:
   a) comment earlier on the said change, in case you might be heading
in the wrong direction
   b) makes it possible to spot potential conflicts earlier
   c) prevent duplicate work in case someone already did this
   d) collaborate (some people helped me test an unfinished PR, which
was nice!)
   e) cheer you up for the progress ;-)

5. If you know who is familiar with a given part of the code, you can
mention them in the PR.

Cheers,

Vincent


More information about the Devel mailing list