<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
span.EmailStyle18
{mso-style-type:personal-reply;
font-family:"Calibri",sans-serif;
color:windowtext;}
.MsoChpDefault
{mso-style-type:export-only;
font-family:"Calibri",sans-serif;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-US" link="blue" vlink="purple" style="word-wrap:break-word">
<div class="WordSection1">
<p class="MsoNormal">I’m not hearing any particular objection to moving to Phabricator-only for reviews. The trend has been in that direction for years anyway. The barriers for one-time contributors are (a) registration, and (b) unfamiliarity with a user-hostile
web UI. Regarding registration, it’s either subscribing to llvm-commits or to Phab; and IIUC, Phab understands github IDs, which makes that barrier pretty low. Regarding the web UI, we have (or did have, admittedly I haven’t looked lately) step-by-step instructions
in the docs for how to navigate Phab successfully, which mitigates (b) as much as we can.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">I am in the camp of, why mess with llvm-commits? The commit emails that Phab sends out already have the commit URL in them right at the top, which looks like it goes to a page where comments can be added (I haven’t actually tried adding
a comment). If those posted comments go to llvm-commits and the author, then I think the only necessary step to turn off email post-commit review is to make llvm-commits read-only except for Phabricator itself.<o:p></o:p></p>
<p class="MsoNormal">We don’t need the ability to have email replies on llvm-commits be recorded in Phab, if we can almost as easily make comments on Phab that go to llvm-commits. Possibly I am overlooking some problem that abandoning llvm-commits entirely
is going to solve?<o:p></o:p></p>
<p class="MsoNormal">--paulr<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<div style="border:none;border-left:solid blue 1.5pt;padding:0in 0in 0in 4.0pt">
<div>
<div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b>From:</b> cfe-dev <cfe-dev-bounces@lists.llvm.org> <b>On Behalf Of
</b>Christian Kühnel via cfe-dev<br>
<b>Sent:</b> Tuesday, May 4, 2021 9:15 AM<br>
<b>To:</b> Aaron Ballman <aaron@aaronballman.com><br>
<b>Cc:</b> llvm-dev <llvm-dev@lists.llvm.org>; clangd-dev@lists.llvm.org; openmp-dev@lists.llvm.org; lldb-dev@lists.llvm.org; cfe-dev@lists.llvm.org; libcxx-dev@lists.llvm.org; flang-dev@lists.llvm.org; parallel_libs-dev@lists.llvm.org<br>
<b>Subject:</b> Re: [cfe-dev] [llvm-dev] [RFC] Deprecate email code reviews in favor of Phabricator<o:p></o:p></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div>
<div>
<p class="MsoNormal">Hi Aaron,<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<p class="MsoNormal">> “Code reviews are conducted on our web-based code-review tool (see Code Reviews with Phabricator).”<br>
<br>
Personally, I am in favor of this policy for initiating code reviews,<br>
but am opposed to it for post-commit feedback. The problem, as I see<br>
it, is that not every change *gets* code review via Phab and the email<br>
lists are the only place on which to provide that post-commit<br>
feedback. <o:p></o:p></p>
</blockquote>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">You can set up notifications on commits in Phabricator. Phabricator adds the user "llvm-commits" as subscriber to certain reviews:
<a href="https://urldefense.com/v3/__https:/reviews.llvm.org/H615__;!!JmoZiZGBv3RvKRSx!p37c8uYDXWnnl1JAGj92AupZsGAGPG0jsP8mxCiLLHtiY0ihBoKrW5hV90ZiYb2jOw$" target="_blank">
https://reviews.llvm.org/H615</a><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">We could do the same thing for commits...<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">So you can reply to any commit via the web UI (or email notification) and the author gets notified as well. One example that worked for me:<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"> <a href="https://urldefense.com/v3/__https:/reviews.llvm.org/rG7f9717b922d421c30f889034488563c67076aca1__;!!JmoZiZGBv3RvKRSx!p37c8uYDXWnnl1JAGj92AupZsGAGPG0jsP8mxCiLLHtiY0ihBoKrW5hV90YQuXXWaw$" target="_blank">https://reviews.llvm.org/rG7f9717b922d421c30f889034488563c67076aca1</a><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<p class="MsoNormal">The Phab commit message does not have any subscribers though, and so<br>
my understanding is that comments on that Phab interface are not<br>
communicated out to the community as a whole, which means the<br>
community may miss out on important post-commit-review information<br>
like general awareness of the problem, workarounds people can use<br>
until the author corrects something, alternative ideas on how to fix<br>
the issue, etc. Or do I misunderstand the way Phab works in this<br>
workflow?<o:p></o:p></p>
</blockquote>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">We can add automatically subscribers to commits as well if we want to. <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">One random example where a subscriber was added to a commit via a Herald rule:<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><a href="https://urldefense.com/v3/__https:/reviews.llvm.org/rG93537fabcee8fcfa3316d7abd5e935f7fe9b468f__;!!JmoZiZGBv3RvKRSx!p37c8uYDXWnnl1JAGj92AupZsGAGPG0jsP8mxCiLLHtiY0ihBoKrW5hV90ZMMYsijg$" target="_blank">https://reviews.llvm.org/rG93537fabcee8fcfa3316d7abd5e935f7fe9b468f</a><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<p class="MsoNormal">Also, "the commenter would need to find the Phabricator commit"<br>
concerns me -- adding extra burden on the people providing feedback<br>
means that *some* amount of those people will struggle enough to<br>
simply not provide that feedback. Responding to an email is about as<br>
low as I think we can set that bar, so the current approach has better<br>
ergonomics for giving feedback. <o:p></o:p></p>
</blockquote>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">If we set up the notifications via Phabricator, you can reply via email. These contain a link at the bottom that will take you directly to the commit page in Phabricator. Not sure why we don't have these on the mailing list...<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<p class="MsoNormal">Tangential complaint -- our use of Herald causes some pain for me as a<br>
list moderator because we've reached the point where Herald<br>
automatically adds so many subscribers to a review that it gets marked<br>
as spam for every email that is generated for the review. <o:p></o:p></p>
</blockquote>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">So this would be a reason to replace the XXX-commits mailing lists with something else...<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<p class="MsoNormal">Given how often Phabricator goes down (which is not super often, but<br>
often enough that people know it happens), I am deeply uncomfortable<br>
with the idea of shutting down the current *-commits mailing lists<br>
because of the chance for data loss. Personally, I think the *-commits<br>
lists are working well and I would prefer they be left alone.<o:p></o:p></p>
</blockquote>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">If Phabricator is down, you're not getting any email notifications from it anyway. So we might already be losing data right now...<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">But fair point: The more we rely on Phabricator, the higher the reliability requirements.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Best,<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">Christian<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</div>
</div>
</div>
</div>
</body>
</html>