[lld] r193585 - [PassManager] add ReaderWriter{Native, YAML} to the Driver.

[Chandler Carruth]

On Tue, Oct 29, 2013 at 10:57 AM, Shankar Easwaran
<shankare at codeaurora.org>wrote:

> Hi Rui,
>
> I thought the procedure was wait for LGTM before you close the review.
>

Correct.

Is this only for reviews that go through phabricator ?
>

No, phabricator is a tool. It has nothing to do with the policy.

The policy is very, very simple: once you ask for a code review, wait for
an explicit 'LGTM' to commit.


> I was referring to this chain where we had agreed for the code to be
> submitted.
>
> http://lists.cs.uiuc.edu/**pipermail/llvm-commits/Week-**
> of-Mon-20131021/192579.html<http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20131021/192579.html>
>

That email chain resulted in several things:

1) Me clarifying that you hadn't completed the previous code review.

2) A discussion about a potential strategy for that patch to go in before
each flavor was fixed to work with this specific behavior.

#2 was not a code review, it was just a technical idea and discussion (and
a good one).

My comments in #1 weren't agreement, and I didn't see anything else in the
thread that would constitute agreement. It appeared at first that you
didn't interpret it as agreement either, as you did exactly the right thing
and started another code review. However, there were (again) comments
outstanding at the time you committed. While there may be early indications
of a path toward a submittable form for the patch, or general support for
the direction of a patch, these aren't the same as having the comments
addressed and an explicit approval to conclude the review.


I think you need to take a step back and slow down a bit. I know you may be
under pressure to implement this quickly, but that isn't a justification
for ignoring or responding poorly to feedback from the community. You've
recently caused folks in the project to spend a significant amount of their
time trying to explain things, unbreak things, etc. That isn't fair to them
or the project as a whole, and it uses up the time that Rui and others
could spend reviewing your patch. I don't want to see this continue, as it
isn't healthy, and is delaying ELF support in LLD.

Instead, I would encourage you to take specific care with each code review,
work to address all of the comments, and wait patiently for review to
complete. That is not only the best way forward and the one required in an
open source project like LLVM, but it will actually be the fastest as well.

Also, you may not realize it, but you are getting some of the fastest code
review turn-around times of any subproject in LLVM. =] I don't think it is
unreasonable to ask that you wait for the reviews prior to moving forward.
This functionality is really important, and I'm excited that you're working
on it, and so I really want to ensure your contributions are effective.

-Chandler
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131029/6a4781f5/attachment.html>