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

On 01/15, Emilio Cobos Álvarez wrote:
> On 1/8/20 2:15 AM, Bill Wendling via cfe-dev wrote:
> > It's not hyperbole, but fine. How do you use it to keep multiple,
> > related changes in order? 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? Why can't I *easily* relate changes to each
other?
> > 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?
> FWIW, Mozilla moved to Phabricator not long ago and the revision stack
thing
> was a huge annoyance.
> 
> Nowadays we have tools to manage the stacks for us [1][2], and that as a
> plus don't require arc/php to be installed on your system.
> 
> I don't do much LLVM / clang stuff, but submitting stuff with [1] just
works
> (with `moz-phab HEAD~N --no-bug`), and it should submit your last N commits
> separately automatically, without having to submit them one-by one and
> linking them via the web interface / annotate stuff in the commit message.
> 
> Sorry if this is just noise, though maybe it helps.
This looks pretty cool, thanks! I'll for sure give it a try!


> 
> [1]: https://github.com/mozilla-conduit/review
> [2]: https://github.com/mystor/phlay
-- 

Johannes Doerfert
Researcher

Argonne National Laboratory
Lemont, IL 60439, USA

jdoerfert at anl.gov
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200115/710009b0/attachment.sig>

"Doerfert, Johannes via llvm-dev" <llvm-dev at lists.llvm.org>
writes:
>> Nowadays we have tools to manage the stacks for us [1][2], and that as
a
>> plus don't require arc/php to be installed on your system.
>> 
>> I don't do much LLVM / clang stuff, but submitting stuff with [1]
just works
>> (with `moz-phab HEAD~N --no-bug`), and it should submit your last N
commits
>> separately automatically, without having to submit them one-by one and
>> linking them via the web interface / annotate stuff in the commit
message.
>> 
>> Sorry if this is just noise, though maybe it helps.
>
> This looks pretty cool, thanks! I'll for sure give it a try!
Agreed, this is interesting as is git-phab.

I would like to take a step back and talk about existing use-cases in
Phab.  People have talked a lot about linking revisions, patch series,
parent/child relationships and so on and I have to confess I am
struggling to understand the use-cases for these things.  Most of what I
do upstream is simple enough to accomplish with individual patch
submissions and reviews.  In some cases I have posted "mega-patches"
for
context purposes but I'll admit that isn't a very good workflow as
things quickly get out of date.

I would like to understand better how people use Phab's advanced
features and why.  For example, what drives the need for patch series
and dependence relationships?  Some walk-through examples would be very
helpful.

                         -David

On 1/15/20 6:50 PM, David Greene wrote:
> "Doerfert, Johannes via llvm-dev" <llvm-dev at
lists.llvm.org> writes:
> 
>>> Nowadays we have tools to manage the stacks for us [1][2], and that
as a
>>> plus don't require arc/php to be installed on your system.
>>>
>>> I don't do much LLVM / clang stuff, but submitting stuff with
[1] just works
>>> (with `moz-phab HEAD~N --no-bug`), and it should submit your last N
commits
>>> separately automatically, without having to submit them one-by one
and
>>> linking them via the web interface / annotate stuff in the commit
message.
>>>
>>> Sorry if this is just noise, though maybe it helps.
>>
>> This looks pretty cool, thanks! I'll for sure give it a try!
> 
> Agreed, this is interesting as is git-phab.
> 
> I would like to take a step back and talk about existing use-cases in
> Phab.  People have talked a lot about linking revisions, patch series,
> parent/child relationships and so on and I have to confess I am
> struggling to understand the use-cases for these things.  Most of what I
> do upstream is simple enough to accomplish with individual patch
> submissions and reviews.  In some cases I have posted
"mega-patches" for
> context purposes but I'll admit that isn't a very good workflow as
> things quickly get out of date.
> 
> I would like to understand better how people use Phab's advanced
> features and why.  For example, what drives the need for patch series
> and dependence relationships?  Some walk-through examples would be very
> helpful.
At least at Mozilla, it's good practice to split a given patch in
logical, reviewable pieces that are associated to the same bug, and that
the same or different people may need to review.

That generally makes the patches get reviewed much sooner. During
review, some of the initial parts may be good to land, while some others
may need changes.

[1] or [2] are recentish examples that come to mind, but it happens
fairly often. Of course for a bunch of simpler changes one revision is
enough.

The use cases are similar to the "I have one PR with multiple commits"
in GitHub, but with the advantage of being able to review them
individually, and thus they can land upstream sooner.

 -- Emilio

Hi David,

On Wed, Jan 15, 2020 at 12:51 PM David Greene via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
> I would like to understand better how people use Phab's advanced
> features and why.  For example, what drives the need for patch series
> and dependence relationships?  Some walk-through examples would be very
> helpful.
Here's a somewhat more complex example of changes made by myself a
year and a half ago, starting with https://reviews.llvm.org/D48013

It's a series of changes across TableGen and the AMDGPU backend to
refactor how we represent a class of instructions. Some patterns that
occur:
- an NFC (no functional change) refactoring/rename change in an area
that is then later touched by a functional change.
- a change to common infrastructure (TableGen) that is motivated by
and used by a subsequent functional change in the AMDGPU backend, but
which can stand on its own and which may want to be reviewed by
different people
- a cleanup change to remove deprecated functionality from the backend
once the refactored functionality is available, separated out in order
to smoothen the upgrade path for frontends that are maintained outside
of the llvm-project repository

Reviewing all of this in a single amorphous diff is something that I
wouldn't wish on anybody. Conversely, having the linkage between
different commits provides context to reviewers.

It is also helpful to me: I can keep track of reviews to the series
while simultaneously working on other changes that happen to be
unrelated, and having the commits in separate stacks helps by
implicitly grouping them. Admittedly this advantage is comparatively
minor because the UI isn't perfect.

Cheers,
Nicolai
-- 
Lerne, wie die Welt wirklich ist,
aber vergiss niemals, wie sie sein sollte.