[asterisk-dev] Proposed Review board Usage policy change

[Russell Bryant]

On Thu, Dec 13, 2012 at 11:58 AM, Mark Michelson <mmichelson at digium.com>wrote:

> At Astricon in October, one policy that came up was the idea to give
> everyone around the world a fair shot at reading the content on the review
> board. As such, I am proposing the following to be added to the
> "Reviewboard Usage" wiki page:
>
> "When someone approves of a review request, they will click the "Ship it!"
> button as a way of signing off on the review. When you have received a
> "Ship it!" you can feel confident that in the opinion of one person, your
> code has met approval.
>
> Since Asterisk is a global project, it is important that people around the
> world get a fair chance to review your code before the review is closed. If
> your code receives a "Ship it!" but has not been on the review board for at
> least 24 hours, please refrain from committing. It may be that a reviewer
> on the opposite side of the planet has a valid objection to your code but
> has not had the chance to express himself/herself. If the code has been up
> for at least 24 hours, it is reasonable to assume that the code has been
> given a fair chance to be reviewed by anyone interested. Ideally, you
> should give another 24 hours after the initial "Ship it!" so that people
> can have one last opportunity to bring up any objections.
>
> Exceptions to this policy may be enacted if the fix is time-sensitive,
> such as if the fix is determined to be security-related or if the fix is
> for a widely-felt catastrophic regression."
>
>
> Please let me know what you think.
>

-1

My opinion is that this is overkill and makes the contribution process
unnecessarily complicated.

If there is a legitimate objection or problem with a commit, it's easy
enough to change further or revert as appropriate.

I think the amount of time a patch needs to be up for comment should be
based on how invasive and/or controversial a change is.  I understand that
not everyone has the same eyes for this sort of thing, but I still don't
think complicating the process is worthwhile.  As examples, the last two
reviews I put up were simple and non-controversial changes.  I don't see
*any* reason to arbitrarily slow down getting a simple patch in like this:

    https://reviewboard.asterisk.org/r/2230/
    https://reviewboard.asterisk.org/r/2245/

Further, if you want to talk about patch process, I think there are much
more important and more fundamental patch process things that should be
documented/decided before discussing such a detail.  Examples:

1) Which patches must go through reviewboard vs JIRA vs both vs nothing
(straight to commit)?

2) Take #1 and consider the process for someone with commit access as well
as someone without.  I get the feeling that the process for submitting
patches as someone without commit access is confusing and unclear right now
because of the multiple paths patches take.

3) When a patch is on reviewboard, what "Ship It" matters?  The tool allows
anyone to say "Ship It", but clearly just anyone shouldn't be able to
approve code going in.

Some of this may seem obvious to people that work full-time on Asterisk
every day, but it's not obvious to newcomers, and it's even not obvious to
me since I'm not looking at Asterisk every day anymore.

-- 
Russell Bryant
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20121213/af4b6984/attachment-0001.htm>