llvm dev - Oct 2012 - [LLVMdev] Announcement: Phabricator for code reviews

Dear LLVM / Clang community,

we'd like to open the use of Phabricator as an optional tool for doing code
reviews to a wider audience. Please feel free to start your code reviews by
following the documentation at http://llvm.org/docs/Phabricator.html.

Note that e-mail is still the reference medium for code reviews. Please let
me know about any problems with Phabricator or the documentation you find.

Cheers,
/Manuel
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20121017/f88ed3c1/attachment.html>

You might want to mention other functionality, like Herald, which I
think fills a need that a lot of developers have.

Well, I guess since this message is on the list, I can just say:
Phabricator's "Herald" tool lets you (among other things) set up
actions to happen in response to certain events; one huge use case for
this is setting up alerts when parts of the tree that you are
interested in watching get changed: this alleviates the problem of
missing the chance to post-commit review on changes due to messages
getting lost in the flood of llvm-commits mails.

There's a lot of other functionality in Phabricator that I think might
be useful; the big question is how to have it integrate properly into
the mailing list---that's the reason I have held back in exploring
some of the features.

-- Sean Silva

On Wed, Oct 17, 2012 at 1:18 PM, Manuel Klimek <klimek at google.com>
wrote:
> Dear LLVM / Clang community,
>
> we'd like to open the use of Phabricator as an optional tool for doing
code
> reviews to a wider audience. Please feel free to start your code reviews by
> following the documentation at http://llvm.org/docs/Phabricator.html.
>
> Note that e-mail is still the reference medium for code reviews. Please let
> me know about any problems with Phabricator or the documentation you find.
>
> Cheers,
> /Manuel
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>

On Wed, Oct 17, 2012 at 8:25 PM, Sean Silva <silvas at purdue.edu> wrote:
> You might want to mention other functionality, like Herald, which I
> think fills a need that a lot of developers have.
>
> Well, I guess since this message is on the list, I can just say:
> Phabricator's "Herald" tool lets you (among other things) set
up
> actions to happen in response to certain events; one huge use case for
> this is setting up alerts when parts of the tree that you are
> interested in watching get changed: this alleviates the problem of
> missing the chance to post-commit review on changes due to messages
> getting lost in the flood of llvm-commits mails.
>
> There's a lot of other functionality in Phabricator that I think might
> be useful; the big question is how to have it integrate properly into
> the mailing list---that's the reason I have held back in exploring
> some of the features.
>
I'd say the biggest impact of a tool like Phabricator is for larger
pre-commit code reviews (for example by answering the question "what has
changed since the last patch"). Thus, that's the workflow we spent some
time tuning. If we see that more people use and like Phabricator, we'll put
more effort into making sure other features are integrated nicely with
llvm's existing workflows.

Cheers,
/Manuel

>
> -- Sean Silva
>
> On Wed, Oct 17, 2012 at 1:18 PM, Manuel Klimek <klimek at google.com>
wrote:
> > Dear LLVM / Clang community,
> >
> > we'd like to open the use of Phabricator as an optional tool for
doing
> code
> > reviews to a wider audience. Please feel free to start your code
reviews
> by
> > following the documentation at http://llvm.org/docs/Phabricator.html.
> >
> > Note that e-mail is still the reference medium for code reviews.
Please
> let
> > me know about any problems with Phabricator or the documentation you
> find.
> >
> > Cheers,
> > /Manuel
> >
> > _______________________________________________
> > cfe-dev mailing list
> > cfe-dev at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20121017/7fa3245e/attachment.html>

Still basically unusable with Apple Mail's obstinate refusal to respect
"In-reply-to" or "References". I'm definitely giving
Phabricator patches less attention because the CCs to cfe-commits are too hard
to follow. I realize that you shouldn't have to change a nice,
standards-conforming tool to fit our closed-source app's
"feature", but...

Any news on turning on auto-"Re" for every reply after the initial
patch posting? I can't think of any ill effects from that, and it matches
what people do for mailing-list reviews anyway.

Thanks,
Jordan


On Oct 17, 2012, at 10:18 , Manuel Klimek <klimek at google.com> wrote:
> Dear LLVM / Clang community,
> 
> we'd like to open the use of Phabricator as an optional tool for doing
code reviews to a wider audience. Please feel free to start your code reviews by
following the documentation at http://llvm.org/docs/Phabricator.html.
> 
> Note that e-mail is still the reference medium for code reviews. Please let
me know about any problems with Phabricator or the documentation you find.
> 
> Cheers,
> /Manuel
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20121017/e8c2ec00/attachment.html>

On Wed, Oct 17, 2012 at 9:05 PM, Jordan Rose <jordan_rose at apple.com>
wrote:
> Still basically unusable with Apple Mail's obstinate refusal to respect
> "In-reply-to" or "References". I'm definitely
giving Phabricator patches
> less attention because the CCs to cfe-commits are too hard to follow. I
> realize that you shouldn't have to change a nice, standards-conforming
tool
> to fit our closed-source app's "feature", but...
>
> Any news on turning on auto-"Re" for every reply *after* the
initial
> patch posting? I can't think of any ill effects from that, and it
matches
> what people do for mailing-list reviews anyway.
>
I just contacted the phab folks and implemented and deployed a fix.

Cheers,
/Manuel

>
> Thanks,
> Jordan
>
>
> On Oct 17, 2012, at 10:18 , Manuel Klimek <klimek at google.com>
wrote:
>
> Dear LLVM / Clang community,
>
> we'd like to open the use of Phabricator as an optional tool for doing
> code reviews to a wider audience. Please feel free to start your code
> reviews by following the documentation at
> http://llvm.org/docs/Phabricator.html.
>
> Note that e-mail is still the reference medium for code reviews. Please
> let me know about any problems with Phabricator or the documentation you
> find.
>
> Cheers,
> /Manuel
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20121017/73daf891/attachment.html>

Hi Manuel,
> we'd like to open the use of Phabricator as an optional tool for doing
code
> reviews to a wider audience. Please feel free to start your code reviews by
> following the documentation at http://llvm.org/docs/Phabricator.html.
sorry for the silly question but... how do you sign up?  The "sign up"
section
doesn't have a "sign up here" link.  It does have a link to
"Code Repository
Browser", which asks for a phabricator, github or google login.  It looks
like
you can register with phabricator using a github or google account; is that
required, or is it possible to sign up for phabricator without going via github
or google?

Thanks for your help, Duncan.

This looks great! One hitch though: it looks like I can monitor files
by name but what I really want to watch is a whole directory
"/llvm/trunk/include/llvm-c". I didn't see any way to do that. Do
you
have any idea if that is possible? I can add a rule for each file
which is probably OK but then I'll miss any new files.

Best, Keith

On Wed, Oct 17, 2012 at 2:25 PM, Sean Silva <silvas at purdue.edu>
wrote:
> You might want to mention other functionality, like Herald, which I
> think fills a need that a lot of developers have.
>
> Well, I guess since this message is on the list, I can just say:
> Phabricator's "Herald" tool lets you (among other things) set
up
> actions to happen in response to certain events; one huge use case for
> this is setting up alerts when parts of the tree that you are
> interested in watching get changed: this alleviates the problem of
> missing the chance to post-commit review on changes due to messages
> getting lost in the flood of llvm-commits mails.
>
> There's a lot of other functionality in Phabricator that I think might
> be useful; the big question is how to have it integrate properly into
> the mailing list---that's the reason I have held back in exploring
> some of the features.
>
> -- Sean Silva
>
> On Wed, Oct 17, 2012 at 1:18 PM, Manuel Klimek <klimek at google.com>
wrote:
>> Dear LLVM / Clang community,
>>
>> we'd like to open the use of Phabricator as an optional tool for
doing code
>> reviews to a wider audience. Please feel free to start your code
reviews by
>> following the documentation at http://llvm.org/docs/Phabricator.html.
>>
>> Note that e-mail is still the reference medium for code reviews. Please
let
>> me know about any problems with Phabricator or the documentation you
find.
>>
>> Cheers,
>> /Manuel
>>
>> _______________________________________________
>> cfe-dev mailing list
>> cfe-dev at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev


-- 
keithsheppard.name

Hi Duncan,

On Thu, Oct 18, 2012 at 1:45 PM, Duncan Sands <baldrick at free.fr> wrote:
> Hi Manuel,
>
>
>  we'd like to open the use of Phabricator as an optional tool for doing
>> code
>> reviews to a wider audience. Please feel free to start your code
reviews
>> by
>> following the documentation at
http://llvm.org/docs/**Phabricator.html<http://llvm.org/docs/Phabricator.html>
>> .
>>
>
> sorry for the silly question but... how do you sign up?  The "sign
up"
> section
> doesn't have a "sign up here" link.  It does have a link to
"Code
> Repository
> Browser", which asks for a phabricator, github or google login.  It
looks
> like
> you can register with phabricator using a github or google account; is that
> required, or is it possible to sign up for phabricator without going via
> github
> or google?
>
> Thanks for your help, Duncan.

The only way to not use an OAuth provider is to have an administrator
create an account for you. I can switch on the facebook OAuth in addition
to google and github, if that would make it easier for you.

If you strongly prefer not to use an OAuth provider, let me know and I can
create a user for you, but I'd be interested in the reasons (and the
upstream phab devs might be, too).

Cheers,
/Manuel
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20121018/0d6209a1/attachment.html>

On Wed, Oct 17, 2012 at 10:18 AM, Manuel Klimek <klimek at google.com>
wrote:
> Dear LLVM / Clang community,
>
> we'd like to open the use of Phabricator as an optional tool for doing
code
> reviews to a wider audience. Please feel free to start your code reviews by
> following the documentation at http://llvm.org/docs/Phabricator.html.
>
> Note that e-mail is still the reference medium for code reviews. Please let
> me know about any problems with Phabricator or the documentation you find.
>
> Cheers,
> /Manuel
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>
One minor issue with the patch I just sent out. If you specify a
specific reviewer, it starts the email with Hi, <name>. Which is
weird. In general review requests are to everyone, with specific
people that you know care about the code in question CCed.

- Michael Spencer

On Fri, Oct 19, 2012 at 1:05 AM, Michael Spencer <bigcheesegs at
gmail.com>wrote:
> On Wed, Oct 17, 2012 at 10:18 AM, Manuel Klimek <klimek at
google.com> wrote:
> > Dear LLVM / Clang community,
> >
> > we'd like to open the use of Phabricator as an optional tool for
doing
> code
> > reviews to a wider audience. Please feel free to start your code
reviews
> by
> > following the documentation at http://llvm.org/docs/Phabricator.html.
> >
> > Note that e-mail is still the reference medium for code reviews.
Please
> let
> > me know about any problems with Phabricator or the documentation you
> find.
> >
> > Cheers,
> > /Manuel
> >
> > _______________________________________________
> > cfe-dev mailing list
> > cfe-dev at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
> >
>
> One minor issue with the patch I just sent out. If you specify a
> specific reviewer, it starts the email with Hi, <name>. Which is
> weird. In general review requests are to everyone, with specific
> people that you know care about the code in question CCed.
>
I added this, as it felt a little more personal to me (yea, I know, I'm
strange).
I can cut it out again if people don't like it...

Anybody else having an opinion?

Cheers,
/Manuel
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20121019/6ff5d211/attachment.html>