[llvm] r200051 - [Mips] TargetStreamer ELF flag Support for default and commandline options.

[Chandler Carruth]

On Mon, Jan 27, 2014 at 8:22 AM, Rafael Espíndola <
rafael.espindola at gmail.com> wrote:

> > 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.
>
> While it is up to mips what internal procedures you guys use, the
> criticism about lack of understanding of the general design of MC
> stands. I would suggest that simply doing reviews in the open would be
> the most productive.


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.

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.


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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140129/2497e065/attachment.html>