[asterisk-dev] Reviewboard Usage Guidelines

[Tilghman Lesher]

On Thursday 09 April 2009 14:05:14 Jeff Peeler wrote:
> On Thu, Apr 9, 2009 at 10:52 AM, Russell Bryant <russell at digium.com> wrote:
> > Greetings,
> >
> > I have added some reviewboard usage guidelines to our doxygen
> > documentation.  You can find the current version of the documentation
> > here:
> >
> > http://www.asterisk.org/doxygen/trunk/Reviewboard.html
> >
> > One notable bit that is not in the guidelines today, but I think should
> > be considered, is guidelines for when it is _required_ for code to be
> > posted for review before being merged.
> >
> > After seeing how many bugs have been found and fixed through in depth
> > code review on reviewboard before making it in to Asterisk, I am
> > inclined to say that I think _all_ non-trivial changes should be put
> > through reviewboard and require at least one sign off from another
> > committer before being merged.
> >
> > What do others think about this?
>
> Sounds like a good idea. Since it already is kind of the non-official
> policy, the only thing that would really be changing is the documentation
> supporting it.

I'm not sure that that's really true, and I think a REQUIREMENT that it be
posted on reviewboard is excessive.  If you want to mandate review, that's
fine, and reviewboard is an excellent tool for that, but for example, if
another committer simply reviewed a patch, I think that's sufficient to meet
the intended criteria.  Add it to the standard template:  "Reviewed by: A,B".

-- 
Tilghman