[Bf-committers] Patch tracker reviewing rules

[Early Ehlinger]

You may want to mention that persistence can go a long way as well.  If you
*really* want to see something in the trunk, prove it by keeping your patch
alive, updated, and synched with the latest versions of Blender over a long
period of time.  This shows the "insiders" that you're committed and are not
going to just dump a patch into the trunk and skip town, leaving them to
maintain the code in your wake.  It also verifies that your code is useful
enough to merit inclusion.

Also, when developers provide feedback, it's best to try to incorporate
their (hopefully constructive) criticism into your patch.  Usually the patch
will be better, you'll learn something along the way, and you'll ease the
maintenance burden for later on.

-- Early

On Wed, Sep 24, 2008 at 6:35 AM, Ton Roosendaal <ton at blender.org> wrote:

> 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
>
> _______________________________________________
> Bf-committers mailing list
> Bf-committers at blender.org
> http://lists.blender.org/mailman/listinfo/bf-committers
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.blender.org/pipermail/bf-committers/attachments/20080924/9033ded4/attachment-0001.htm