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

[Xinliang David Li via llvm-dev]

On Tue, Mar 8, 2016 at 9:35 PM, Mehdi Amini <mehdi.amini at apple.com> wrote:

>
> 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> 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.
>>
>
> 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.
>
>
This is a very good example of well thought/concrete concerns that can
trigger great discussions. Holding off the feature for discussion in this
scenario is well justified and the absolute the right thing to do.

David


>
> 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/fd924d95/attachment.html>