Hi, I just want to add another point:
2014-03-17 10:21 GMT+01:00 Vincent Petry <[email protected]>: > > 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. 6. Add a Kanban label: Either "5- To review" if the PR is ready for review and doesn't need any changes or "4 - Developing" if it's a work in progress PR or there were issues and comments which aren't resolved/implemented. An keep in mind to update the label. Then it's easier to find an appropiate PR if you've got time for a review and doesn't have to search for a "ready to review" PR - you just need to filter for it [1]. I've done it for the first page of PRs in the last days, when I checked them anyway. Thanks, Morris [1] https://github.com/owncloud/core/issues?labels=5+-+To+review
_______________________________________________ Devel mailing list [email protected] http://mailman.owncloud.org/mailman/listinfo/devel
