[llvm-dev] Formalize "revert for more design review" policy.

[Hal Finkel via llvm-dev]

----- Original Message -----
> From: "Sean Silva" <chisophugis at gmail.com>
> To: "llvm-dev" <llvm-dev at lists.llvm.org>
> Cc: "Chris Lattner" <clattner at apple.com>, "Rafael Ávila de Espíndola" <rafael.espindola at gmail.com>, "Michael Spencer"
> <bigcheesegs at gmail.com>, "Chandler Carruth" <chandlerc at gmail.com>, "David Li" <davidxl at google.com>, "David Blaikie"
> <dblaikie at gmail.com>, "Daniel Berlin" <dberlin at dberlin.org>, "Easwaran Raman" <eraman at google.com>, "Hal Finkel"
> <hfinkel at anl.gov>
> Sent: Tuesday, March 8, 2016 11:03:09 PM
> Subject: Re: Formalize "revert for more design review" policy.
> 
> 
> +Hal, who somehow slipped through the cracks.
> 

Thanks :-)

> 
> On Tue, Mar 8, 2016 at 9:00 PM, Sean Silva < chisophugis at gmail.com >
> wrote:
> 
> 
> 
> Recently there's been some friction over reversions (I can remember
> two cases in recent memory). In both issues the general feel I got
> is that as a community we should honor "revert for more design
> review" requests unconditionally.
> 
> 
> What do you guys think of adding something like this to
> DeveloperPolicy.rst as an item at the end of the numbered list in
> http://llvm.org/docs/DeveloperPolicy.html#code-reviews ?
> 
> 
> 
> #. Sometimes patches get committed that need more discussion.
> If a developer thinks that a patch would benefit from some more
> review
> and promptly communicates this, the patch should be reverted
> (preferably
> by the original author, unless they are unresponsive).
> Developers often disagree, and erring on the side of the developer
> asking for more review prevents any lingering disagreement over code
> in
> the tree.
> 
> 
> "promptly" is there mostly to avoid suggesting a "necro-revert"; once
> the code has been in tree for long enough at some point it would be
> more appropriate to open a bug report or start a fresh discussion.
> 
> 
> "unresponsive" add some nebulousness, but I think it's an important
> exception to call out for the "preferably by the original author".
> 

I think this generally sounds right, and matches what we currently expect in practice. To this we might add that it is then the responsibility of the developer requesting the revert to ensure that the review happens promptly.

Do we have a formal policy for other kinds of reverts? We also generally revert if someone claims to have bisected a miscompile to a particular commit, and this is true even if a test case is not immediately available (although we do need some kind of reproducer before too long).

 -Hal

> 
> 
> -- Sean Silva
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory