<div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Oct 29, 2013 at 10:57 AM, Shankar Easwaran <span dir="ltr"><<a href="mailto:shankare@codeaurora.org" target="_blank" class="cremed">shankare@codeaurora.org</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div id=":613" style="overflow:hidden">Hi Rui,<br>
<br>
I thought the procedure was wait for LGTM before you close the review.</div></blockquote><div><br></div><div>Correct.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div id=":613" style="overflow:hidden">
Is this only for reviews that go through phabricator ?<br></div></blockquote><div><br></div><div>No, phabricator is a tool. It has nothing to do with the policy.</div><div><br></div><div>The policy is very, very simple: once you ask for a code review, wait for an explicit 'LGTM' to commit.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div id=":613" style="overflow:hidden">I was referring to this chain where we had agreed for the code to be submitted.<br>

<br>
<a href="http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20131021/192579.html" target="_blank" class="cremed">http://lists.cs.uiuc.edu/<u></u>pipermail/llvm-commits/Week-<u></u>of-Mon-20131021/192579.html</a></div>
</blockquote></div><br></div><div class="gmail_extra">That email chain resulted in several things:</div><div class="gmail_extra"><br></div><div class="gmail_extra">1) Me clarifying that you hadn't completed the previous code review.</div>
<div class="gmail_extra"><br></div><div class="gmail_extra">2) A discussion about a potential strategy for that patch to go in before each flavor was fixed to work with this specific behavior.</div><div class="gmail_extra">
<br></div><div class="gmail_extra">#2 was not a code review, it was just a technical idea and discussion (and a good one).</div><div class="gmail_extra"><br></div><div class="gmail_extra">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.</div>
<div class="gmail_extra"><br></div><div class="gmail_extra"><br></div><div class="gmail_extra">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.</div>
<div class="gmail_extra"><br></div><div class="gmail_extra">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.</div>
<div class="gmail_extra"><br></div><div class="gmail_extra">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.</div>
<div class="gmail_extra"><br></div><div class="gmail_extra">-Chandler</div></div>