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

[Shankar Easwaran]

On 10/29/2013 6:18 PM, Chandler Carruth wrote:
> 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.
Now I am confused, whats the policy on code thats committed without 
reviews ? What I see is we follow a post-commit review culture.
> 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).
#2 was part of a code review chain.
>
> 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.
The reason I started a complete different chain was the original review 
chain became too long, and thought this review request could get lost in 
the whole thread.
>
> 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.
This patch didnt break anything.  Essentially did what was discussed at 
#2. This has slowed me down a lot :)

I am still waiting for review comments that I missed handling:(

> 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.
Sure.

Thanks

Shankar Easwaran

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by the Linux Foundation