[llvm-dev] Anyone doing code reviews via mailing lists?

Philip Reames via llvm-dev
2021-04-23T17:25:58Z

If you want to propose this, please move it to a top level thread for
visibility.

(My thoughts inline to help you refine your proposal.)

On 4/23/21 9:55 AM, Krzysztof Parzyszek via llvm-dev
wrote:
>
> I think we should phase-out email reviews in favor of doing it
> directly via Phabricator.
>
> My reasons:
>
>  1. Phabricator allows for both pre- and post-commit reviews.  You can
>     “raise concern” with any commit, including those that did not have
>     a pre-commit review on phab.
>  2. The email-phabricator integration is still deficient despite a lot
>     of effort having been put into it.  Moreover, it’s not likely that
>     it will ever be fully functional.
>  3. Email communication is “fragile”. This one is based more on my
>     personal experience, but even simply following a discussion on
>     llvm-dev has been difficult.  I now have to use Outlook (corporate
>     reasons…) and Outlook fails at keeping email threads together.
>      Replies to the same tread are scattered into a mini-forest
>     instead staying as a single tree.  There are issues with every
>     email client formatting the replies differently: top-post mixed
>     with bottom-post mixed with inline text, with people quoting 500
>     lines of text only to insert a single-line response, and so on,
>     and so forth. Some of it is due to my use of Outlook, some of it
>     is independent from it.  This “infinite flexibility” of email
>     structure is the reason why I doubt that the Phabricator
>     integration will ever work.
>
Phabricator is also fragile unfortunately.  I find phabricators inline
discussions to be very hard to follow for anything which becomes
involved.  As such, I tend to default to phab, but quickly move to email
if discussion gets complicated.
>
>  1. Phabricator’s interface makes every review look the same, is
>     readable, and doesn’t make it easy to unintentionally clutter it
>     with junk.
>
As someone with vision restrictions, please be careful about this line
of argument.  One of the major advantages of email is that I can use my
own client at whatever zoom/scale I want.  Phab "sorta works" from an
accessibility perspective, but frankly is inferior to plain old email.
I frequently end up reading phabricator emails, and then replying
through the web interface.

I'm just point this out because I find that visual appear is often rated
much more highly by some folks than others. Personally, functionality is
pretty much the only thing I care about.

>      1. Finally, it seems like nowadays it’s easier to create a
>         Phabricator account than to sign up to the mailing lists…
>
> --
>
> Krzysztof Parzyszek kparzysz at quicinc.com <mailto:kparzysz at
quicinc.com>
> AI tools development
>
> *From:* llvm-dev <llvm-dev-bounces at lists.llvm.org> *On Behalf Of
> *Mehdi AMINI via llvm-dev
> *Sent:* Friday, April 23, 2021 11:01 AM
> *To:* Hubert Tong <hubert.reinterpretcast at gmail.com>
> *Cc:* llvm-dev <llvm-dev at lists.llvm.org>
> *Subject:* [EXT] Re: [llvm-dev] Anyone doing code reviews via mailing
> lists?
>
> On Fri, Apr 23, 2021 at 8:57 AM Hubert Tong via llvm-dev
> <llvm-dev at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>> wrote:
>
>     On Fri, Apr 23, 2021 at 11:25 AM Christian Kühnel via llvm-dev
>     <llvm-dev at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>> wrote:
>
>         So it looks like the information we have on the mailing list
>         and in Phabricator is diverging, as those emails do not get
>         parsed back into Phabricator.
>
>     I recall noticing that Phabricator also doesn't emit inline code
>     change suggestions into the e-mail record.
>
> Yes: which is an indication that not all the content is on the
> mailing-list either :(
>
> --
>
> Mehdi
>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> 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/20210423/2722b858/attachment.html>