[PATCH] Tooling: Call-back for begin/end of sources for newFrontendActionFactory

Manuel Klimek klimek at google.com
Mon Jun 3 08:25:48 PDT 2013


On Mon, Jun 3, 2013 at 4:58 PM, Vane, Edwin <edwin.vane at intel.com> wrote:

> Alright, point taken. It was just my feeling from past review requests and
> comments from others saying this community is generally in favour of
> post-commit reviews that I thought this was the right course of action.
> I'll take this to mean explicit directions are required to change to
> post-commit.
>

Yes, the community is generally all right with post-commit review, and you
should use good judgement to figure out when it's not needed. The
interesting part is that once you decide that a change needs pre-commit
review (usually because of being unsure about a certain part of it), you're
usually right :) Always feel free to ping me about changes, or let me know
if changes are the blocking part for something else you work on, so I can
prioritize them.

Cheers,
/Manuel




>
> > -----Original Message-----
> > From: Manuel Klimek [mailto:klimek at google.com]
> > Sent: Monday, June 03, 2013 9:02 AM
> > To: reviews+D882+public+891a0a7537cd3886 at llvm-reviews.chandlerc.com
> > Cc: Daniel Jasper; Douglas Gregor; Vane, Edwin; cfe-commits at cs.uiuc.edu;
> > Chandler Carruth
> > Subject: Re: [PATCH] Tooling: Call-back for begin/end of sources for
> > newFrontendActionFactory
> >
> > On Wed, May 29, 2013 at 6:03 PM, Edwin Vane <edwin.vane at intel.com>
> wrote:
> >
> >
> >
> >         Moving to post-commit review.
> >
> >
> >
> > Just FYI: this is usually a discouraged pattern :) If you have the
> feeling that
> > something needs pre-commit review once, there are very few things that
> would
> > make it actually useful to switch to post-commit (apart from doug
> telling you on
> > IRC, which you would then note in the review)
> >
> > Cheers,
> > /Manuel
> >
> >
> >
> >       http://llvm-reviews.chandlerc.com/D882
> >
> >       BRANCH
> >         callbacks
> >
> >       ARCANIST PROJECT
> >         clang
> >
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130603/f450b78b/attachment.html>


More information about the cfe-commits mailing list