Ack! Sorry! 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. My deepest apologies. Shall not happen again. Will continue the wait. Sincerely, Faisal
Sent from my iPhone On Aug 22, 2013, at 7:20 AM, Manuel Klimek <[email protected]> wrote: > On Wed, Aug 21, 2013 at 7:53 PM, Faisal Vali <[email protected]> wrote: > > 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 ... > thank you! > > Hi Faisal, > > 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. > > 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). > > In this case, we noticed that the change went in only because our tools noted > a layering violation: > Decl should not depend on Sema. > > 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. > > 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. > > Cheers, > /Manuel
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
