A policy for reviewboard

Felix Rohrbach fxrh at gmx.de
Mon Jul 18 15:53:24 CEST 2011


Hi there,

Currently, I'm trying to get my changes of my SoK project into master. I'm 
using reviewboard for that and I'm missing a clear policy there when I can 
merge my commit into master. As the other students will also need to get their 
patches through a review process and as we might get new contributors on the 
Desktop Summit, it is imho a good idea to create a clear policy for that. 
There was some discussion about that on IRC, too.

One example of such a policy:
http://community.kde.org/Calligra/Policies/Review_board_rules

Things we should discuss imo:
1. Types of review requests
2. Number of ship-its
3. Time

My proposal:
1. I would differentiate between two types of review requests. The first one, 
the minor request, is a simple change that clearly has no side effects and is 
unlikely to lead to discussions between developers. In this category are not 
too complex bug fixes as well as small features like adding a menu action. 
Everything else would be a major review request.

2. For minor changes, one should be enough, major request should normally wait 
for two ship-its. These numbers can change if a reviewer for example requests 
more ship-its or if he says that he is the developer who wrote or maintains 
this part of the project alone (then you wouldn't need two requests for a 
major request).

3. I think (and the Calligra people seem to do, too) that we should not let 
contributors wait too long, so I think a request should be seen as accepted 
after a certain time without any new changes or comments. We can fix issues 
also if they are already in master, even if that should not happen too often. 
Time counts from the last comment or change on the review request, but only if 
all the requests of the reviewer were implemented in the patch. If a reviewer 
needs more time (or if someone else knows that a dev who should review this is 
on vacation), the time can be increased. This would be my suggestion:
Minor: 2 days (maybe this is a bit short for those of you who already work?)
Major: 1 week

What do you think about this?

Regards,
Felix


More information about the Gluon mailing list