<div dir="ltr">On Tue, Oct 29, 2013 at 4:50 PM, Shankar Easwaran <span dir="ltr"><<a href="mailto:shankare@codeaurora.org" target="_blank">shankare@codeaurora.org</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">On 10/29/2013 6:18 PM, Chandler Carruth wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
No, phabricator is a tool. It has nothing to do with the policy.<br>
<br>
The policy is very, very simple: once you ask for a code review, wait for<br>
an explicit 'LGTM' to commit.<br>
</blockquote></div>
Now I am confused, whats the policy on code thats committed without reviews ? What I see is we follow a post-commit review culture.<div class="im"><br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
2) A discussion about a potential strategy for that patch to go in before<br>
each flavor was fixed to work with this specific behavior.<br>
<br>
#2 was not a code review, it was just a technical idea and discussion (and<br>
a good one).<br>
</blockquote></div>
#2 was part of a code review chain.<div class="im"><br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
My comments in #1 weren't agreement, and I didn't see anything else in the<br>
thread that would constitute agreement. It appeared at first that you<br>
didn't interpret it as agreement either, as you did exactly the right thing<br>
and started another code review. However, there were (again) comments<br>
outstanding at the time you committed. While there may be early indications<br>
of a path toward a submittable form for the patch, or general support for<br>
the direction of a patch, these aren't the same as having the comments<br>
addressed and an explicit approval to conclude the review.<br>
</blockquote></div>
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.<div class="im"><br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
I think you need to take a step back and slow down a bit. I know you may be<br>
under pressure to implement this quickly, but that isn't a justification<br>
for ignoring or responding poorly to feedback from the community. You've<br>
recently caused folks in the project to spend a significant amount of their<br>
time trying to explain things, unbreak things, etc. That isn't fair to them<br>
or the project as a whole, and it uses up the time that Rui and others<br>
could spend reviewing your patch. I don't want to see this continue, as it<br>
isn't healthy, and is delaying ELF support in LLD.<br>
</blockquote></div>
This patch didnt break anything. Essentially did what was discussed at #2. This has slowed me down a lot :)<br>
<br>
I am still waiting for review comments that I missed handling:(</blockquote><div><br></div><div>Apparently the thing that's missed in the code review was the unused field _file. You removed it after submitting the code, fine, but that was a open question when you submitted without LGTM. There was a design discussion regarding FileToMutable class (I still don't like it) was going on, as you once thought my suggestion would be better than FileToMutable, so there really was an open question.</div>
<div><br></div><div>And one thing is that I mentioned about the unused field three times in the code review. The last six emails between you and me were to repeat the same thing about the dead field and suggesting a better design. You've lost your time by rushing to commit it. Please take your time to read my comments and your code. That would actually make the process faster.</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Instead, I would encourage you to take specific care with each code review,<br>
work to address all of the comments, and wait patiently for review to<br>
complete. That is not only the best way forward and the one required in an<br>
open source project like LLVM, but it will actually be the fastest as well.<br>
</blockquote></div>
Sure.<br>
<br>
Thanks<span class="HOEnZb"><font color="#888888"><br>
<br>
Shankar Easwaran</font></span><div class="HOEnZb"><div class="h5"><br>
<br>
-- <br>
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by the Linux Foundation<br>
<br>
</div></div></blockquote></div><br></div></div>