<div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr">Hi,<div><br></div><div>It seems like there was a general agreement in this thread, and our current written guidelines say "In addition, if substantial problems are identified, it is expected that the patch is reverted and fixed offline", which may not be explicit enough.</div><div><br></div><div>With the previous suggestions from Philip's incorporated in Sean's proposal, I amended the doc here: <a href="https://reviews.llvm.org/D89995">https://reviews.llvm.org/D89995</a></div><div><br></div><div>Thanks,</div><div><br></div><div>-- </div><div>Mehdi</div><div><br></div><div><br></div><div><br></div><div><div> <br></div></div></div></div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Mar 11, 2016 at 7:46 AM Renato Golin via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">I agree with the paragraph, and most of the comments thereafter. The<br>
intention to clarify reverts as an evolutionary process instead of a<br>
punishment is most welcome.<br>
<br>
Without reverts we can't have post-commit reviews in any way,<br>
especially with so many downstream projects depending on LLVM.<br>
<br>
Code and bot owners may be more trigger happy than other members, but<br>
that's because they care about the overall status of their part in the<br>
compiler and/or their architectures, and that's an important point of<br>
view that should not be taken lightly.<br>
<br>
I believe our community works mostly well in regards to commits,<br>
reviews and reverts, but it's good to encode at least the general idea<br>
so that new members don't feel bad about having their patches<br>
reverted.<br>
<br>
cheers,<br>
-renato<br>
<br>
On 9 March 2016 at 12:00, Sean Silva via llvm-dev<br>
<<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br>
> Recently there's been some friction over reversions (I can remember two<br>
> cases in recent memory). In both issues the general feel I got is that as a<br>
> community we should honor "revert for more design review" requests<br>
> unconditionally.<br>
><br>
> What do you guys think of adding something like this to DeveloperPolicy.rst<br>
> as an item at the end of the numbered list in<br>
> <a href="http://llvm.org/docs/DeveloperPolicy.html#code-reviews" rel="noreferrer" target="_blank">http://llvm.org/docs/DeveloperPolicy.html#code-reviews</a> ?<br>
><br>
> #. Sometimes patches get committed that need more discussion.<br>
>    If a developer thinks that a patch would benefit from some more review<br>
>    and promptly communicates this, the patch should be reverted (preferably<br>
>    by the original author, unless they are unresponsive).<br>
>    Developers often disagree, and erring on the side of the developer<br>
>    asking for more review prevents any lingering disagreement over code in<br>
>    the tree.<br>
><br>
> "promptly" is there mostly to avoid suggesting a "necro-revert"; once the<br>
> code has been in tree for long enough at some point it would be more<br>
> appropriate to open a bug report or start a fresh discussion.<br>
><br>
> "unresponsive" add some nebulousness, but I think it's an important<br>
> exception to call out for the "preferably by the original author".<br>
><br>
> -- Sean Silva<br>
><br>
> _______________________________________________<br>
> LLVM Developers mailing list<br>
> <a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
><br>
_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
</blockquote></div>