[owncloud-devel] Pull Requests suggested guidelines

Frank Karlitschek frank at owncloud.org
Mon Mar 17 12:25:44 MET 2014


I think this makes a lot of sense.
We should put this in the developer documentation.


Frank

On 17.03.2014, at 10:21, Vincent Petry <pvince81 at owncloud.com> wrote:

> 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
> _______________________________________________
> Devel mailing list
> Devel at owncloud.org
> http://mailman.owncloud.org/mailman/listinfo/devel



More information about the Devel mailing list