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

Manuel Jacob via cfe-dev cfe-dev at lists.llvm.org
Fri Jun 26 01:53:18 PDT 2020


On 2020-06-26 09:53, James Henderson via llvm-dev wrote:
> Relatedly, Phabricator doesn't stop you continuing a comment chain for
> reasons I have yet to follow, which Github sometimes does.
> 
> Some others:
> 1) I believe Github also doesn't have an easy way to respond to 
> multiple
> comments simultaneously, if you are not in "review" mode, (which is 
> always
> the case if you are replying to out-of-line comments).
> 2) Typically in our Phabricator, the description and summary of a patch 
> is
> considered to be roughly what goes in the final commit message (same 
> with
> Github). However, updating said commit message isn't possible to my
> knowledge without force-pushing in Github (which as discussed elsewhere 
> in
> the thread causes other problems).
> 3) Marking comments as "Done" in Phabricator does not auto-hide them,
> whereas in Github marking a comment as "Resolved" does. Spoken from
> experience, this leads to situations in Github where a developer thinks
> they've resolved a reviewers comments, marks them as Resolved, but 
> actually
> they weren't. The reviewer in turn then has to browse the comments in 
> the
> summary page, to see if they have any unaddressed comments that were
> supposedly resolved, which given the limited context available there is 
> a
> non-trivial task sometimes.

In practice, I’ve found that these two functionalities are used in 
different ways. In Phabricator, the implementer marks the comments as 
done. In GitHub, the reviewer marks comments are resolved. Actually, I 
would like to have both, but I’m not aware of a review software that has 
both.

> There are probably others, but those are the ones not already mentioned
> that I can come up with so far.
> 
> P.S. Thanks Mehdi/Fangrui for stepping up! Whilst I'm not opposed to
> working with Github towards getting the feature parity with Phabricator
> sorted, I don't want to switch until the two are on a par with each 
> other,
> which in my opinion is not yet, so being forced to do so prematurely 
> due to
> lack of maintainers would have made me very sad (P.P.S thank you Manuel 
> for
> Phabricator in the first place!).
> 
> On Thu, 25 Jun 2020 at 22:39, Hubert Tong via cfe-dev <
> cfe-dev at lists.llvm.org> wrote:
> 
>> On Thu, Jun 25, 2020 at 12:29 PM Mehdi AMINI via llvm-dev <
>> llvm-dev at lists.llvm.org> wrote:
>> 
>>> 
>>> Re: patch chains - perhaps we can ask github on support for that?
>>>> I think what would help is somebody providing a doc with the 
>>>> features
>>>> that phab provides that github PR doesn't, so we can get some 
>>>> consensus on
>>>> what the diff is.
>>>> 
>>> 
>>> Indeed, I'll try to start a doc on this.
>>> 
>> Thanks. Some things that are high on my list:
>> Phabricator has persistent per-user inline comment collapsing.
>> Phabricator can show all inline comments over the lifetime of the 
>> patch
>> in-line (even if the placement is not perfect) in the full diff view.
>> 
>> _______________________________________________
>> 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


More information about the cfe-dev mailing list