[llvm-dev] [cfe-dev] Phabricator Maintenance

[Fangrui Song via llvm-dev]

On 2020-06-26, Mehdi AMINI via llvm-dev wrote:
>On Fri, Jun 26, 2020 at 12:41 PM Danila Malyutin via llvm-dev <
>llvm-dev at lists.llvm.org> wrote:
>
>> For the record,
>> Phabricator is even worse for that purpose as it doesn't really guarantee
>> that what's been reviewed is actually what will be committed.
>> In fact, I've got bit by that before svn->git transition (error in
>> reapplying the patch on svn for submission).
>> GitHub PRs solve this problem. In fact, there wouldn't be any need for the
>> "getting commit rights" process and all those pings from the new
>> contributors, since it'd be easy to make PR mergeable by anyone as long
>> once they are approved.
>>

It is sometimes handy that the commit can have a slightly different
message from the review "summary". One can make a final adjustment to
the message. The reviewers/subscribers can get one less email
notification.

With github, one may have to do a force push to adjust the description
("Commits" pulls/*/commits, not "Conversation"). However, as previous
discussions mentioned, force push can cause stray comment issues.

I also want to note that with the "Squash and merge" button, it is
easily messing up the commit message with bullet points such as "Fix ..."
"Lowercase ..." "Typo" ...

>I think merging a PR on GitHub requires permission on the repo: a new
>contributor would require someone with "commit rights" to click on the
>merge button for them.
>
>
>
>> Although some of the friction disappeared after the move to git and after
>> I discovered moz-phab.
>>
>> --
>> Danila
>>
>> > -----Original Message-----
>> > From: llvm-dev [mailto:llvm-dev-bounces at lists.llvm.org] On Behalf Of
>> Nicolai
>> > Hahnle via llvm-dev
>> > Sent: Thursday, June 25, 2020 18:10
>> > To: Nikita Popov <nikita.ppv at gmail.com>
>> > Cc: LLVM Dev <llvm-dev at lists.llvm.org>; Clang Dev <
>> cfe-dev at lists.llvm.org>
>> > Subject: Re: [llvm-dev] [cfe-dev] Phabricator Maintenance
>> >
>> > On Thu, Jun 25, 2020 at 11:43 AM Nikita Popov via llvm-dev <llvm-
>> > dev at lists.llvm.org> wrote:
>> > > On Thu, Jun 25, 2020 at 11:22 AM Zachary Turner via llvm-dev <llvm-
>> > dev at lists.llvm.org> wrote:
>> > >> What this means for LLVM is that everyone will have to completely
>> stop using
>> > history rewriting operations.  No more rebase, squash, amend, etc.
>> > >
>> > > This is also incorrect. Most GitHub projects I work on use a
>> rebase-oriented
>> > workflow without issue, so clearly that's possible...
>> > >
>> > > The only issue in this area that I'm aware of is that you can't easily
>> see what
>> > changed between two revisions of a pull request if you both a) rebase
>> onto
>> > master and b) amend commits at the *same* time. For review purposes it is
>> > most useful if additional changes are not squashed into existing
>> commits, but
>> > pushed on top. The squash is best performed when landing the changes.
>> >
>> > Disagree. I usually want to be able to review the actual commits that
>> will be
>> > landed, not some imagined combination that may or may not line up with
>> what
>> > the author will end up squashing things to.
>> >
>> > Remember, the goal is for authors to be able to have a review of multiple
>> > commits in a patch series, i.e.
>> >
>> >    Patch 1
>> >    Patch 2
>> >
>> > ... and then make changes to both of those patches based on reviews.
>> > If you just push delta commits, then in the best case, if people
>> properly use `git
>> > commit --squash`, you get:
>> >
>> >    Patch 1
>> >    Patch 2
>> >    squash! Patch 1
>> >    squash! Patch 2
>> >
>> > or something along those lines, and things just become really messy to
>> keep
>> > track of. The Phabricator Stacks UI isn't ideal for this, but it tends
>> to work quite a
>> > bit better than what GitHub has.
>> >
>> > Cheers,
>> > Nicolai
>> > --
>> > Lerne, wie die Welt wirklich ist,
>> > aber vergiss niemals, wie sie sein sollte.
>> > _______________________________________________
>> > LLVM Developers mailing list
>> > llvm-dev at lists.llvm.org
>> > https://urldefense.com/v3/__https://lists.llvm.org/cgi-
>> > bin/mailman/listinfo/llvm-
>> > dev__;!!A4F2R9G_pg!OD1Fs4auqBkrZx526L0axvXPTf5YH9MrHI_UReeJz9VA6Nu-
>> > VQi8Ibwu6fVE0w0$
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>

>_______________________________________________
>LLVM Developers mailing list
>llvm-dev at lists.llvm.org
>https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev