[PATCH] Fix dominator descendants for unreachable blocks.

[Renato Golin]

Hi Chandler,

You're right. I was merely repeating a pattern I've seen happening without
much thought. Will be less hasty in the future.

Cheers,
Renato
On 30 Nov 2013 00:44, "Chandler Carruth" <chandlerc at gmail.com> wrote:

>
> On Fri, Nov 29, 2013 at 9:47 AM, Renato Golin <renato.golin at linaro.org>wrote:
>
>> But since this patch is really obvious, and because I'm sure you ran
>> all the tests, you should be able to commit if no one else reply to it
>> in a day or so, and we can do post-commit review.
>>
>
> Totally off the topic of *this* patch, but I wanted to reply directly to
> this comment.
>
> I'm somewhat firmly opposed to this practice, and have said so repeatedly
> on the list. Once someone has decided to mail out a patch for pre-commit
> review, it was insufficiently obvious *to them* for post-commit review.
> Stick with the review, and get an LGTM. Trivial patches are fast to review,
> and so it is exceedingly rare that these actually cause the big bottlenecks
> in the community's review process.
>
> Renato, the obviousness of the patch to you may make you comfortable
> giving an LGTM outside of your area, and that seems totally fine (although
> note that I spotted a bug in the patch). However, I don't ever see a reason
> for "if no one else replies in a day or so" or "wait a few days and then go
> ahead"[1]. The fact that a reviewer (or author) feels the need to wait is
> an indication that they aren't confident in their review and thus someone
> else should provide the final review. This doesn't mean that this kind of
> early review isn't useful -- it totally is!!! A decent chunk of the time it
> can spot obvious goofs quickly and early, and let the author update for
> them. It can reduce the cycles of review when a person who knows the area
> shows up and can look at it. It also helps build the reviewer's experience
> in the area (and this is exactly how a i learned several areas of LLVM and
> Clang!) so don't take this as discouraging these reviews at all. They are
> awesome, thank you for doing them and keep doing them! =]
>
> It's just that ultimately, unlike wine, patches don't get better with age.
> ;]
>
> Anyways, I'll get off my soapbox and go do something useful.
> -Chandler
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131130/b8ce1a45/attachment.html>