[Bf-committers] Patch tracker reviewing rules

[Ton Roosendaal]

Hi all,

Here's a proposal for how to make patch reviewing easier, and better 
communicated.
Below is a text proposal to include on the submission page:

--------------------------------------------

1) Submission guidelines

Submitting patches in this tracker are very welcome! To make timely and 
proper reviews possible, we'd recommend you to check on the guidelines 
below.

- Patches solving bugs or compiling errors will usually get handled 
quickly. Note that for such patches we typically need similar info as 
for bug reports, which you can find on this page <link>. Especially for 
more complex fixes we really need to be able to verify that ourselves.

- Patches providing new features are more complicated to handle. A 
review has to be done both on functional level (does this fit with 
Blender's design or roadmap) and technical level (is the provided code 
conform our coding style, matches design and planning, and is 
sufficiently readable and maintainable).

- For larger patches it's relevant to note if you seek one of the 
Blender project members to submit and maintain it, or whether you 
propose to get involved as team member to maintain/improve it in the 
future.

- Provide as much relevant documentation as possible. Images, 
screenshots, short video clips or .blend file examples do do wonders! 
Attach all of this in the tracker itself, or a website which you know 
will remain accessible for a long while.


2) How to get a review

Doing a good review can take a lot of time, and because almost all 
regular Blender developers are volunteers, we cannot make a promise 
you'll get such a review here! Below are tips how to get reviews done:

- The most efficient way is to contact the maintainers of the code 
yourself. You could actually do this in advance even, to verify if your 
proposed work would make a good chance for inclusion. You can best find 
maintainers of code by reviewing svn commit history in svn <link>. Also 
check on the maintainers page <link> and consider to join one of our 
communication channels via the Get Involved page <link>.

- When the patch has actual new functionality, user reviews will help a 
lot too. A lot of patches were tested first by providing public builds 
via websites like graphicall.org, or via forums on www.blender.org or 
www.blenderartists.org.

3) The review process

We recommend every of the active Blender developers to spend some time 
on reviewing patches. In the past we've done that a lot, and with a bit 
of luck you'll get great comments and advise for how to improve it. 
However... to prevent people to feel obliged to spend time on lengthy 
review discussions we also add a grading convention:

- A patch review can be a simple comment only saying "+1" (I like it) 
or "-1" (I don't like it). No justifications are needed for that, nor a 
note whether this is about functionality or implementation.

- Getting three "+1" marks from current bf-blender team members <link> 
means that your patch gets the highest priority for review, discussed 
in our weekly irc meetings and assigned to one of the maintainers for a 
serious review.
Other people's votes will only be viewed as an indication.

- Getting three "-1" marks from current bf-blender team members will 
allow us to close the patch without further explanation.

- When positive or negative marks both occur, we'll sum the total and 
apply the rules above.

4) Help my patch got rejected!

First of all, we're all software developers here, and we understand 
code work on Blender shows a lot of commitment, and has been your time 
investment with all the good intentions to help us out. A rejection 
doesn't mean we don't like you, nor that we don't like to see actively 
helping out as a contributor! It's all human beings here, with personal 
preferences, and ideas for how to make Blender a better product. It 
makes Blender as an open source project strong if we allow active 
developers and maintainers to make individual choices too. They're 
getting empowered to do development, which also implies they have get 
the benefit of the doubt in making tough decisions.
Nevertheless, mistakes can always happen! Here's what to do to get a 
patch considered after all:

- Contact the developer list with a friendly request for (additional) 
reviews. Make sure the current maintainer of code at least is aware of 
this rejection.

- If no result or consensus in the project team can be found, you can 
ask the 'admins' of bf-blender to provide a final verdict (Matt, 
Willian, Martin, Ton).

-----------------------------



-Ton-

------------------------------------------------------------------------
Ton Roosendaal  Blender Foundation   ton at blender.org    www.blender.org
Blender Institute BV  Entrepotdok 57A  1018AD Amsterdam The Netherlands