llvm dev - Jan 2020 - [cfe-dev] Phabricator -> GitHub PRs?

I'm not sure a decision was already made as such. I think it's more that
there was a flurry of conversation last time with lots of conflicting opinions,
and then the conversation just fizzled out.

FWIW, I like Phabricator but I'm willing to try GitHub. Overall I think we
should take the same approach that eventually led to Phabricator being widely
adopted: We should allow GitHub PR's and see if the community generally
settles on one or the other.
> On Jan 7, 2020, at 19:13, Bill Wendling <isanbard at gmail.com>
wrote:
> 
> Thanks for the tip.
> 
> I didn't know that the decision had already been made to stay with
Phabricator. I find Phabricator hard to use and it makes me actively avoid
contributing to LLVM knowing that I'll have to use it. I've read the
documentation, but still have issues. And it's not like git where
there's an obscure command not everyone knows about that will fix the
problem. The advice I got so far has been to use Phabricator/Arcanist as little
as possible within my workflow. I'll try doing that.
> 
> I apologize to anyone I may have offended.
> 
> -bw
> 
> On Tue, Jan 7, 2020 at 6:26 PM Daniel Sanders <daniel_l_sanders at
apple.com <mailto:daniel_l_sanders at apple.com>> wrote:
> 
> 
> > On Jan 7, 2020, at 17:35, Jonas Devlieghere via llvm-dev <llvm-dev
at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote:
> > 
> > On Tue, Jan 7, 2020 at 5:16 PM Bill Wendling via cfe-dev
> > <cfe-dev at lists.llvm.org <mailto:cfe-dev at
lists.llvm.org>> wrote:
> >> 
> >> On Tue, Jan 7, 2020 at 4:59 PM Doerfert, Johannes <jdoerfert at
anl.gov <mailto:jdoerfert at anl.gov>> wrote:
> >>> 
> >>> Hi Bill,
> >>> 
> >>> On 01/07, Bill Wendling via llvm-dev wrote:
> >>>> Then perhaps those opposed could suggest how to use
Phabricator/Arcanist so
> >>>> that I don't throw my keyboard through my monitor?
> >>> 
> >>> Please explain your problems, w/o the hyperbole, so people can
actually do that.
> >>> 
> >> It's not hyperbole, but fine.  How do you use it to keep
multiple, related changes in order?
> > 
> > You can use parent/child revisions. Phabricator encourages a
> > patches-based approach with small changes. For me that corresponds to
> > one commit per code review. When I address code review feedback in a
> > parent revision I use git's interactive rebase.
> 
> It's worth mentioning that Phabricator can read strings of the format
'Depends on D1234' from commit messages and create those relationships
for you.
> 
> Also just because it's not easy to find unless you know it's there.
You can view the parent/child relationships in the 'Revision Contents'
section on the 'Stack' tab.
> 
> >> The interface for reviewing and responding to reviews is horrific,
e.g. quoting text from a review is rather bad, the email it sends is badly
formatted and hard to read. How do you make it reasonably useful?
> > 
> > Inline comments are super useful, they can be marked as done and
> > hidden. I agree that sometimes there's a lot of context when
quoting
> > text, but the format is very simple (similar to e-mail) so it's
easy
> > to trim.
> > 
> >> Why can't I *easily* relate changes to each other?
> > 
> > What issues do you experience with parent/child revisions?
> > 
> >> Why can't I submit through the Phabricator interface, but have
to go to the command line, place the change in a new branch, pull to
top-of-tree, rebase, and only then push while hoping it doesn't give fail
because the tree became out of date? How can I do a rebase through Phabricator?
> > 
> > You can upload patches through
> > https://reviews.llvm.org/differential/diff/create/
<https://reviews.llvm.org/differential/diff/create/>. I personally
don't
> > use arcanist even though I found it pretty useful in the past.
> > 
> >> 
> >> These are only off the top of my head. There are far more problems
I've had with them.
> >> 
> >> -bw
> >> _______________________________________________
> >> cfe-dev mailing list
> >> cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>
> >> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
<https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev>
> > _______________________________________________
> > LLVM Developers mailing list
> > llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
<https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>
> 
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200107/a3c3f59e/attachment.html>

On Tue, Jan 7, 2020 at 7:32 PM Daniel Sanders
<daniel_l_sanders at apple.com> wrote:
>
> I'm not sure a decision was already made as such. I think it's more
that there was a flurry of conversation last time with lots of conflicting
opinions, and then the conversation just fizzled out.
>
> FWIW, I like Phabricator but I'm willing to try GitHub. Overall I think
we should take the same approach that eventually led to Phabricator being widely
adopted: We should allow GitHub PR's and see if the community generally
settles on one or the other.
+1
>
> On Jan 7, 2020, at 19:13, Bill Wendling <isanbard at gmail.com>
wrote:
>
> Thanks for the tip.
>
> I didn't know that the decision had already been made to stay with
Phabricator. I find Phabricator hard to use and it makes me actively avoid
contributing to LLVM knowing that I'll have to use it. I've read the
documentation, but still have issues. And it's not like git where
there's an obscure command not everyone knows about that will fix the
problem. The advice I got so far has been to use Phabricator/Arcanist as little
as possible within my workflow. I'll try doing that.
>
I believe that technically sending patches to the mailing list is
still a valid way to get your code reviewed. Not everyone monitors the
mailing list actively though so that might turn out to be more
frustrating than Phabricator.
> I apologize to anyone I may have offended.
>
> -bw
>
> On Tue, Jan 7, 2020 at 6:26 PM Daniel Sanders <daniel_l_sanders at
apple.com> wrote:
>>
>>
>>
>> > On Jan 7, 2020, at 17:35, Jonas Devlieghere via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
>> >
>> > On Tue, Jan 7, 2020 at 5:16 PM Bill Wendling via cfe-dev
>> > <cfe-dev at lists.llvm.org> wrote:
>> >>
>> >> On Tue, Jan 7, 2020 at 4:59 PM Doerfert, Johannes
<jdoerfert at anl.gov> wrote:
>> >>>
>> >>> Hi Bill,
>> >>>
>> >>> On 01/07, Bill Wendling via llvm-dev wrote:
>> >>>> Then perhaps those opposed could suggest how to use
Phabricator/Arcanist so
>> >>>> that I don't throw my keyboard through my monitor?
>> >>>
>> >>> Please explain your problems, w/o the hyperbole, so people
can actually do that.
>> >>>
>> >> It's not hyperbole, but fine.  How do you use it to keep
multiple, related changes in order?
>> >
>> > You can use parent/child revisions. Phabricator encourages a
>> > patches-based approach with small changes. For me that corresponds
to
>> > one commit per code review. When I address code review feedback in
a
>> > parent revision I use git's interactive rebase.
>>
>> It's worth mentioning that Phabricator can read strings of the
format 'Depends on D1234' from commit messages and create those
relationships for you.
>>
>> Also just because it's not easy to find unless you know it's
there. You can view the parent/child relationships in the 'Revision
Contents' section on the 'Stack' tab.
>>
>> >> The interface for reviewing and responding to reviews is
horrific, e.g. quoting text from a review is rather bad, the email it sends is
badly formatted and hard to read. How do you make it reasonably useful?
>> >
>> > Inline comments are super useful, they can be marked as done and
>> > hidden. I agree that sometimes there's a lot of context when
quoting
>> > text, but the format is very simple (similar to e-mail) so
it's easy
>> > to trim.
>> >
>> >> Why can't I *easily* relate changes to each other?
>> >
>> > What issues do you experience with parent/child revisions?
>> >
>> >> Why can't I submit through the Phabricator interface, but
have to go to the command line, place the change in a new branch, pull to
top-of-tree, rebase, and only then push while hoping it doesn't give fail
because the tree became out of date? How can I do a rebase through Phabricator?
>> >
>> > You can upload patches through
>> > https://reviews.llvm.org/differential/diff/create/. I personally
don't
>> > use arcanist even though I found it pretty useful in the past.
>> >
>> >>
>> >> These are only off the top of my head. There are far more
problems I've had with them.
>> >>
>> >> -bw
>> >> _______________________________________________
>> >> cfe-dev mailing list
>> >> cfe-dev at lists.llvm.org
>> >> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>> > _______________________________________________
>> > LLVM Developers mailing list
>> > llvm-dev at lists.llvm.org
>> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
>

On Tue, Jan 7, 2020 at 10:36 PM Jonas Devlieghere via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> On Tue, Jan 7, 2020 at 7:32 PM Daniel Sanders
> <daniel_l_sanders at apple.com> wrote:
> >
> > I'm not sure a decision was already made as such. I think it's
more that
> there was a flurry of conversation last time with lots of conflicting
> opinions, and then the conversation just fizzled out.
> >
> > FWIW, I like Phabricator but I'm willing to try GitHub. Overall I
think
> we should take the same approach that eventually led to Phabricator being
> widely adopted: We should allow GitHub PR's and see if the community
> generally settles on one or the other.
>
This means that people proposing patches control the apparent behaviour.
How is someone that is primarily a reviewer meant to voice their opinion
under such a system?
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200107/3557cd74/attachment.html>

On Tue, Jan 7, 2020, 19:33 Daniel Sanders via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> I'm not sure a decision was already made as such. I think it's more
that
> there was a flurry of conversation last time with lots of conflicting
> opinions, and then the conversation just fizzled out.
>
> FWIW, I like Phabricator but I'm willing to try GitHub. Overall I think
we
> should take the same approach that eventually led to Phabricator being
> widely adopted: We should allow GitHub PR's and see if the community
> generally settles on one or the other.
>
Sounds good to me as long as there is a way to contribute that doesn't
require a GitHub account, since there are people who can't/won't use
GitHub
for various reasons.

Jacob
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200107/75ac2fda/attachment.html>

> -----Original Message-----
> From: cfe-dev <cfe-dev-bounces at lists.llvm.org> On Behalf Of John
Marshall
> via cfe-dev
> Sent: Friday, January 31, 2020 7:04 AM
> To: Jonas Devlieghere via cfe-dev <cfe-dev at lists.llvm.org>
> Subject: Re: [cfe-dev] [llvm-dev] Phabricator -> GitHub PRs?
> 
> On 8 Jan 2020, Jonas Devlieghere wrote:
> > I believe that technically sending patches to the mailing list is
> > still a valid way to get your code reviewed. Not everyone monitors the
> > mailing list actively though so that might turn out to be more
> > frustrating than Phabricator.
> 
> I can confirm that this is indeed frustrating.
> 
> I am only a user of Clang (and a former very minor contributor to GCC) but
> I was recently sufficiently piqued by a small Clang diagnostic infelicity
> that I looked into fixing it, and came up with what appears to this
> neophyte to be a trivial 2-line fix. As a first-time contributor to Clang,
> I read the instructions for contributing at
> <http://clang.llvm.org/get_involved.html>:
> 
> 	"Clang is a subproject of the LLVM Project, but has its own mailing
> lists because the communities have people with different interests. The
> two clang lists are:
> 	• cfe-commits - This list is for patch submission/discussion.
> 	[snip]"
> 
> And at <https://llvm.org/docs/GettingStarted.html#sending-patches>
(via
> <http://clang.llvm.org/hacking.html#patches>: "To contribute
changes to
> Clang see LLVM's Getting Started page"):
> 
> 	"We don’t currently accept github pull requests, so you’ll need to
> send patches either via emailing to llvm-commits, or, preferably, via
> Phabricator."
> 
> Having a trivial one-off patch to propose, and presented with a choice of
> creating a Phabricator account at llvm and learning how to use it or
> simply sending the patch via email -- obviously I chose the latter [1].
> It's only been 10 days but there have been no replies and around 2000
> other emails on the list since then. Of those ~2000, I noticed three that
> were not automatically generated -- one of which was a reply to another
> newbie, so well done Jonas Toth! [2]
> 
> Apart from that one instance of a reply, it would appear that 99+% of the
> messages on cfe-commits these days are automatically generated and hence
> that approximately zero people are actively monitoring the mailing list.
> So it would probably be good to update the contributing instructions to
> reflect reality.
> 
>     John
I expect 99+% of the messages on cfe-commits are automatically generated,
but that doesn't mean nobody reads the list.  I'm not the only one who
finds the Phabricator UI to be appallingly bad or even impenetrable, for
anything more sophisticated than posting comments.  (I also have a recipe
for posting new patches, learned through trial and many errors.)
I certainly don't use the web UI for figuring out which patches to read
and/or comment on; I use the mailing list for that.  Regretfully I don't
do much with the Clang sub-project.

The protocol for proposed patches is effectively the same for emailed
patches as for Phab patches: directly CC people who would appear to be
appropriate reviewers, and reply with a "ping" every week or so if
there
are no responses.  This will bump the patch up in the mailing list queue
on the list, and (one hopes) the direct CC will be noticed by people who
don't ordinarily read the list.

HTH,
--paulr
> 
> 
> [1] http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-
> 20200120/302838.html
> [2] http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-
> 20200127/304742.html
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

On Fri, Jan 31, 2020 at 6:09 AM Robinson, Paul via cfe-dev <
cfe-dev at lists.llvm.org> wrote:
>
>
> > -----Original Message-----
> > From: cfe-dev <cfe-dev-bounces at lists.llvm.org> On Behalf Of
John
> Marshall
> > via cfe-dev
> > Sent: Friday, January 31, 2020 7:04 AM
> > To: Jonas Devlieghere via cfe-dev <cfe-dev at lists.llvm.org>
> > Subject: Re: [cfe-dev] [llvm-dev] Phabricator -> GitHub PRs?
> >
> > On 8 Jan 2020, Jonas Devlieghere wrote:
> > > I believe that technically sending patches to the mailing list is
> > > still a valid way to get your code reviewed. Not everyone
monitors the
> > > mailing list actively though so that might turn out to be more
> > > frustrating than Phabricator.
> >
> > I can confirm that this is indeed frustrating.
> >
> > I am only a user of Clang (and a former very minor contributor to GCC)
> but
> > I was recently sufficiently piqued by a small Clang diagnostic
infelicity
> > that I looked into fixing it, and came up with what appears to this
> > neophyte to be a trivial 2-line fix. As a first-time contributor to
> Clang,
> > I read the instructions for contributing at
> > <http://clang.llvm.org/get_involved.html>:
> >
> >       "Clang is a subproject of the LLVM Project, but has its own
mailing
> > lists because the communities have people with different interests.
The
> > two clang lists are:
> >       • cfe-commits - This list is for patch submission/discussion.
> >       [snip]"
> >
> > And at
<https://llvm.org/docs/GettingStarted.html#sending-patches> (via
> > <http://clang.llvm.org/hacking.html#patches>: "To
contribute changes to
> > Clang see LLVM's Getting Started page"):
> >
> >       "We don’t currently accept github pull requests, so you’ll
need to
> > send patches either via emailing to llvm-commits, or, preferably, via
> > Phabricator."
> >
> > Having a trivial one-off patch to propose, and presented with a choice
of
> > creating a Phabricator account at llvm and learning how to use it or
> > simply sending the patch via email -- obviously I chose the latter
[1].
> > It's only been 10 days but there have been no replies and around
2000
> > other emails on the list since then. Of those ~2000, I noticed three
that
> > were not automatically generated -- one of which was a reply to
another
> > newbie, so well done Jonas Toth! [2]
> >
> > Apart from that one instance of a reply, it would appear that 99+% of
the
> > messages on cfe-commits these days are automatically generated and
hence
> > that approximately zero people are actively monitoring the mailing
list.
> > So it would probably be good to update the contributing instructions
to
> > reflect reality.
> >
> >     John
>
> I expect 99+% of the messages on cfe-commits are automatically generated,
> but that doesn't mean nobody reads the list.  I'm not the only one
who
> finds the Phabricator UI to be appallingly bad or even impenetrable, for
> anything more sophisticated than posting comments.  (I also have a recipe
> for posting new patches, learned through trial and many errors.)
> I certainly don't use the web UI for figuring out which patches to read
> and/or comment on; I use the mailing list for that.  Regretfully I
don't
> do much with the Clang sub-project.
>
> The protocol for proposed patches is effectively the same for emailed
> patches as for Phab patches: directly CC people who would appear to be
> appropriate reviewers, and reply with a "ping" every week or so
if there
> are no responses.  This will bump the patch up in the mailing list queue
> on the list, and (one hopes) the direct CC will be noticed by people who
> don't ordinarily read the list.
>
+1 to all that from me - I don't use Phab to manage my review queue - I use
the mailing list. I do skim through all the commits lists on a weekly
(well, I think it's been a couple of weeks now) basis & try to CC
relevant
people on reviews if they're not something I have the time/knowledge to
look at, etc.

- Dave
>
> HTH,
> --paulr
>
> >
> >
> > [1] http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-
> > 20200120/302838.html
> > [2] http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-
> > 20200127/304742.html
> > _______________________________________________
> > cfe-dev mailing list
> > cfe-dev at lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200131/c91ba7f0/attachment.html>