<div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Jan 27, 2014 at 8:22 AM, Rafael Espíndola <span dir="ltr"><<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">> To be fair to Jack, this commit was reviewed internally by myself, Matheus Almeida, and Vladimir Medic before commit. I mention this because I don't think it's fair that Jack takes all of the criticism while we stand by and watch.<br>

<br>
</div>While it is up to mips what internal procedures you guys use, the<br>
criticism about lack of understanding of the general design of MC<br>
stands. I would suggest that simply doing reviews in the open would be<br>
the most productive.</blockquote></div><br>While it is a small tangent, this is something that has come up several times in other contexts just recently and I want to make a point of repeating it here. This isn't to specific single anyone out, but to comment on the general subject matter of internal reviews.</div>
<div class="gmail_extra"><br></div><div class="gmail_extra">Internal code reviews don't count as LLVM code reviews. If folks want to review code internally, they obviously can, but at no point should that constitute review as laid out by the developer policy. Sometimes there is review that happens off list or people make mistakes with "reply-all" vs. "reply" etc., but that shouldn't invalidate the principle that if the author has any reason to need "review before commit" or "commit after approval" (because they are new or don't fully understand something) that review and/or approval *needs* to happen on the public list so that the rest of the community is aware of what is happening.</div>
<div class="gmail_extra"><br></div><div class="gmail_extra"><br></div><div class="gmail_extra">Beyond that, I would encourage all of the corporate contributors to conduct the overwhelming majority of their review on the list. It helps spread knowledge and record insights into the development and evolution of a patch. It helps break down the perception that the first version of a patch is perfect. It helps the entire community feel like patches can and should be thoroughly reviewed pre- or post-commit by seeing such reviews take place. I think this is really beneficial to the health of the community.</div>
</div>