<div dir="ltr">On Thu, Aug 22, 2013 at 3:21 PM, Faisalv <span dir="ltr"><<a href="mailto:faisalv@gmail.com" target="_blank">faisalv@gmail.com</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 bgcolor="#FFFFFF"><div>Ack! Sorry!</div><div>This patch has been silently roasting in review limbo for over a month (after requested technical changes were made) and a mistaken interpretation of the silence allowed my impatience to get the better of me. </div>
<div>My deepest apologies.  Shall not happen again. Will continue the wait.</div></div></blockquote><div><br></div><div>I have a patch out to update the dev policy with more constructive ideas on how to speed up reviews - as it can be quite frustrating at times (we've all been there ;)</div>
<div><br></div><div>Cheers,</div><div>/Manuel</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div bgcolor="#FFFFFF"><div>Sincerely,</div><div>Faisal</div>
<div><br>Sent from my iPhone</div><div><div class="h5"><div><br>On Aug 22, 2013, at 7:20 AM, Manuel Klimek <<a href="mailto:klimek@google.com" target="_blank">klimek@google.com</a>> wrote:<br><br></div><div></div><blockquote type="cite">
<div><div dir="ltr">On Wed, Aug 21, 2013 at 7:53 PM, Faisal Vali <span dir="ltr"><<a href="mailto:faisalv@yahoo.com" target="_blank">faisalv@yahoo.com</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"><br>
  I plan on committing this patch later today - unless a reviewer needs more time or anyone has a good reason for me not to do so ...<br>
  thank you!<br></blockquote><div><br></div><div>Hi Faisal,</div><div><br></div><div>it doesn't look to me like this patch has gotten the necessary "looks good" that would make it ok to check it in - if you got that via a different channel (for example, from a code owner on IRC), please always make sure to mention that on the review thread.</div>


<div><br></div><div>Especially for major changes like this, it is really important to have the code thoroughly reviewed before checking it in. Companies run continuously integrated build environments from clang's development head, so it's critical for us to have a high quality standard for check-ins, otherwise we're wasting people's time to hunt down the issues after they got checked in (which is usually much harder and more expensive than finding issues during review).</div>


<div><br></div><div>In this case, we noticed that the change went in only because our tools noted a layering violation:</div><div>Decl should not depend on Sema.</div><div><br></div><div>After syncing with Chandler on IRC he OK'ed that I roll back this change (and the clean-up follow-up patch). I'll post on the relevant commit threads once that has happened.</div>


<div><br></div><div>I'll try to come up with a proposal for the llvm dev policy to capture the usual rant that Chandler would put at the end of such messages.</div><div><br></div><div>Cheers,</div><div>/Manuel</div></div>

</div></div>
</div></blockquote></div></div></div></blockquote></div><br></div></div>