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

Fri Apr 23 13:35:11 PDT 2021

Yeah, I can't remember the last time I did a review by email. But I'm sure
that they were replies to a commit that broke our bots, and not an actual
review of a pull request.

Worse, I think if someone does a review by email and doesn't copy me
directly, I won't even notice.

On Fri, 23 Apr 2021, 19:08 Chris Lattner via llvm-dev, <
llvm-dev at lists.llvm.org> wrote:

> I think it makes sense to phase out the email review path as well - it is
> better to have a single way to do things, and Phabricator is where our
> center of gravity is.
>
> -Chris
>
> On Apr 23, 2021, at 10:57 AM, Krzysztof Parzyszek via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
> Thanks for the feedback.  I didn’t take accessibility concerns into
> account.  I’ll make sure to mention it if I decide to create a proposal
> (which seems likely).
>
> --
> Krzysztof Parzyszek  kparzysz at quicinc.com   AI tools development
>
> *From:* Philip Reames <listmail at philipreames.com>
> *Sent:* Friday, April 23, 2021 12:26 PM
> *To:* Krzysztof Parzyszek <kparzysz at quicinc.com>; llvm-dev <
> llvm-dev at lists.llvm.org>
> *Subject:* [EXT] Re: [llvm-dev] Anyone doing code reviews via mailing
> lists?
>
>
> 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   AI tools development
>
> *From:* llvm-dev <llvm-dev-bounces at lists.llvm.org>
> <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>
> <hubert.reinterpretcast at gmail.com>
> *Cc:* llvm-dev <llvm-dev at lists.llvm.org> <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> wrote:
>
> On Fri, Apr 23, 2021 at 11:25 AM Christian Kühnel via llvm-dev <
> 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
>
> _______________________________________________
> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20210423/1ebf3f4a/attachment.html>