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

[Mehdi Amini via llvm-dev]

> On Mar 8, 2016, at 9:30 PM, Xinliang David Li via llvm-dev <llvm-dev at lists.llvm.org> wrote:
> 
> 
> 
> On Tue, Mar 8, 2016 at 9:00 PM, Sean Silva <chisophugis at gmail.com <mailto: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 <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.
> 
> This is an interesting proposal. In practice, what I have seen is that developers usually send out RFC (design of some kinds) to llvm-dev long before the actual implementation, and the patch is submitted after RFC did not get any objections. Would the late request like this come as a surprise to the developer? 

One data point: "FP Environment and Rounding mode handling" (see http://lists.llvm.org/pipermail/llvm-dev/2016-February/094869.html ).
There was a discussion on LLVM Dev, then a bunch of patches went through code review for months, and finally Chandler chimed in very late and we totally blocked the feature on a design ground.
It can be painful, it is case-by-case tradeoff to evaluate of course, but this is also what makes LLVM robust and relatively clean IMO.


FWIW I like Sean proposal, and I agree with David about the "priority increase" glitch.


-- 
Mehdi



> 
> thanks,
> 
> David
> 
> 
> 
>  
> 
> "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".
> 
> -- Sean Silva
> 
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160308/6968cdc9/attachment.html>