[llvm] r210414 - Do materialize for floating point

Chandler Carruth chandlerc at google.com
Sun Jun 8 11:42:33 PDT 2014


On Sun, Jun 8, 2014 at 11:26 AM, Daniel Sanders <Daniel.Sanders at imgtec.com>
wrote:

> > > One other misconception that I need to clear up is that our pre-commit
> > > reviews in Phabricator are actually intended for public review. It is
> > > intended to create public-accessible record that a review happened
> > > along with the content of that review. We then commit under the 'you
> > > can commit to code you maintain without pre-commit review' rule (rule
> > > 3 of
> > > http://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access). As
> > > far as the LLVM community is concerned, it is unreviewed at this point
> > > but it is expected that the availability of the pre-commit review
> > > makes most post-commit reviews trivial. Previously, we were trying to
> > > do this kind of thing with our public-accessible-yet-internal bugzilla
> > > but even though that system created a review that was publicly
> > > visible, the LLVM community had no way of finding the review and
> > > therefore it effectively did not exist.
> > >
> >
> > So, that workflow is equivalent to reviewing on the office whiteboard,
> then
> > sharing a snapshot of the whiteboard in the commit log, right?
>
> Yes, that's a pretty good analogy.
>
> > I personally think that workflow you describe *is* acceptable to give a
> little
> > transparency where there's otherwise none, though others here may
> > disagree..
> >
> > The caveat is that those whiteboard snapshots will be naturally subject
> to
> > *greater* scrutiny than usual. They're the sum of your work, your
> portfolio
> > and review board all in one and a way of saying "we're skilled enough
> that
> > we pre-screen our own patches".
> >
> > If you get it right, it's an opportunity to show off the effectiveness
> of your
> > internal process in coordination with post-commit review. But if that
> > whiteboard is full of unrecognisable scrawls ending in doodles and
> "LGTM",
> > not so good ;-)
>

I agree that this seems acceptable.

But here is a question for you: what harm is caused by CC-ing llvm-commits
from the beginning? If you're going to do a pre-commit review anyways, why
not do it on the list?

One reason why I encourage this (specifically, *if* you're going to do a
pre-commit review at all, doing it entirely on the list) is because it
creates a more inclusive and engaged community. It also lets community
members recognize those providing code review, which is a crazy important
task and something that is hard to get visibility into already.

-Chandler
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140608/a5a954ec/attachment.html>


More information about the llvm-commits mailing list