<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=iso-8859-1">
<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;}
@font-face
{font-family:Consolas;
panose-1:2 11 6 9 2 2 4 3 2 4;}
/* Style Definitions */
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:#0563C1;
text-decoration:underline;}
p.MsoPlainText, li.MsoPlainText, div.MsoPlainText
{mso-style-priority:99;
mso-style-link:"Plain Text Char";
margin:0in;
font-size:10.0pt;
font-family:Consolas;}
span.PlainTextChar
{mso-style-name:"Plain Text Char";
mso-style-priority:99;
mso-style-link:"Plain Text";
font-family:Consolas;}
.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;}
/* List Definitions */
@list l0
{mso-list-id:637614844;
mso-list-type:hybrid;
mso-list-template-ids:366358514 67698703 67698713 67698715 67698703 67698713 67698715 67698703 67698713 67698715;}
@list l0:level1
{mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;}
@list l0:level2
{mso-level-number-format:alpha-lower;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;}
@list l0:level3
{mso-level-number-format:roman-lower;
mso-level-tab-stop:none;
mso-level-number-position:right;
text-indent:-9.0pt;}
@list l0:level4
{mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;}
@list l0:level5
{mso-level-number-format:alpha-lower;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;}
@list l0:level6
{mso-level-number-format:roman-lower;
mso-level-tab-stop:none;
mso-level-number-position:right;
text-indent:-9.0pt;}
@list l0:level7
{mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;}
@list l0:level8
{mso-level-number-format:alpha-lower;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;}
@list l0:level9
{mso-level-number-format:roman-lower;
mso-level-tab-stop:none;
mso-level-number-position:right;
text-indent:-9.0pt;}
@list l1
{mso-list-id:1376587127;
mso-list-type:hybrid;
mso-list-template-ids:548815092 67698703 67698713 67698715 67698703 67698713 67698715 67698703 67698713 67698715;}
@list l1:level1
{mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;}
@list l1:level2
{mso-level-number-format:alpha-lower;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;}
@list l1:level3
{mso-level-number-format:roman-lower;
mso-level-tab-stop:none;
mso-level-number-position:right;
text-indent:-9.0pt;}
@list l1:level4
{mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;}
@list l1:level5
{mso-level-number-format:alpha-lower;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;}
@list l1:level6
{mso-level-number-format:roman-lower;
mso-level-tab-stop:none;
mso-level-number-position:right;
text-indent:-9.0pt;}
@list l1:level7
{mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;}
@list l1:level8
{mso-level-number-format:alpha-lower;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;}
@list l1:level9
{mso-level-number-format:roman-lower;
mso-level-tab-stop:none;
mso-level-number-position:right;
text-indent:-9.0pt;}
@list l2
{mso-list-id:1583753958;
mso-list-type:hybrid;
mso-list-template-ids:-648794640 67698703 67698713 67698715 67698703 67698713 67698715 67698703 67698713 67698715;}
@list l2:level1
{mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;}
@list l2:level2
{mso-level-number-format:alpha-lower;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;}
@list l2:level3
{mso-level-number-format:roman-lower;
mso-level-tab-stop:none;
mso-level-number-position:right;
text-indent:-9.0pt;}
@list l2:level4
{mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;}
@list l2:level5
{mso-level-number-format:alpha-lower;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;}
@list l2:level6
{mso-level-number-format:roman-lower;
mso-level-tab-stop:none;
mso-level-number-position:right;
text-indent:-9.0pt;}
@list l2:level7
{mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;}
@list l2:level8
{mso-level-number-format:alpha-lower;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;}
@list l2:level9
{mso-level-number-format:roman-lower;
mso-level-tab-stop:none;
mso-level-number-position:right;
text-indent:-9.0pt;}
@list l3
{mso-list-id:1783264225;
mso-list-type:hybrid;
mso-list-template-ids:-1933568078 67698703 67698713 67698715 67698703 67698713 67698715 67698703 67698713 67698715;}
@list l3:level1
{mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;}
@list l3:level2
{mso-level-number-format:alpha-lower;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;}
@list l3:level3
{mso-level-number-format:roman-lower;
mso-level-tab-stop:none;
mso-level-number-position:right;
text-indent:-9.0pt;}
@list l3:level4
{mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;}
@list l3:level5
{mso-level-number-format:alpha-lower;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;}
@list l3:level6
{mso-level-number-format:roman-lower;
mso-level-tab-stop:none;
mso-level-number-position:right;
text-indent:-9.0pt;}
@list l3:level7
{mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;}
@list l3:level8
{mso-level-number-format:alpha-lower;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;}
@list l3:level9
{mso-level-number-format:roman-lower;
mso-level-tab-stop:none;
mso-level-number-position:right;
text-indent:-9.0pt;}
ol
{margin-bottom:0in;}
ul
{margin-bottom:0in;}
--></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="#0563C1" vlink="#954F72" style="word-wrap:break-word">
<div class="WordSection1">
<p class="MsoPlainText"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">This is a revision of the previous RFC[1]. This RFC limits the scope to pre-commit reviews only.<o:p></o:p></span></p>
<p class="MsoPlainText"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif"><o:p> </o:p></span></p>
<p class="MsoPlainText"><b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">Statement:<o:p></o:p></span></b></p>
<p class="MsoPlainText"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">Our current code review policy states[2]:<o:p></o:p></span></p>
<p class="MsoPlainText" style="margin-left:.5in"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">“Code reviews are conducted, in order of preference, on our web-based code-review tool (see Code Reviews with Phabricator), by email on the relevant
project’s commit mailing list, on the project’s development list, or on the bug tracker.”<o:p></o:p></span></p>
<p class="MsoPlainText"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">This proposal is to limit pre-commit code reviews only to Phabricator. This would apply to all projects in the LLVM monorepo. With the change in effect, the amended policy
would read:<o:p></o:p></span></p>
<p class="MsoPlainText" style="margin-left:.5in"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">“Pre-commit code reviews are conducted on our web-based code-review tool (see Code Reviews with Phabricator). Post-commit reviews are conducted,
in order of preference, on Phabricator, by email on the relevant project’s commit mailing list, on the project’s development list, or on the bug tracker.”<o:p></o:p></span></p>
<p class="MsoPlainText"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif"><o:p> </o:p></span></p>
<p class="MsoPlainText"><b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">Current situation:<o:p></o:p></span></b></p>
<ol style="margin-top:0in" start="1" type="1">
<li class="MsoPlainText" style="mso-list:l2 level1 lfo1"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">In a recent llvm-dev thread[3], Christian Kühnel pointed out that pre-commit code reviews rarely originate via an email (most are started
on Phabricator), although, as others pointed out, email responses to an ongoing review are not uncommon. (That thread also contains examples of mishaps related to the email-Phabricator interactions, or email handling itself.)<o:p></o:p></span></li><li class="MsoPlainText" style="mso-list:l2 level1 lfo1"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">We have Phabricator patches that automatically apply email comments to the Phabricator reviews, although reportedly this functionality is
not fully reliable[4,5]. This can cause review comments to be lost in the email traffic.<o:p></o:p></span></li></ol>
<p class="MsoPlainText"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif"><o:p> </o:p></span></p>
<p class="MsoPlainText"><b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">Benefits:<o:p></o:p></span></b></p>
<ol style="margin-top:0in" start="1" type="1">
<li class="MsoPlainText" style="mso-list:l1 level1 lfo2"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">Single way of doing pre-commit code reviews: these code reviews are a key part of the development process, and having one way of performing
them would make the process clearer and unambiguous.<o:p></o:p></span></li><li class="MsoPlainText" style="mso-list:l1 level1 lfo2"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">Review authors and reviewers would only need to monitor one source of comments without the fear that a review comment may end up overlooked.<o:p></o:p></span></li><li class="MsoPlainText" style="mso-list:l1 level1 lfo2"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">This change</span><span style="font-size:11.0pt;font-family:"Calibri",sans-serif"> simply codifies an existing practice.</span><span style="font-size:11.0pt;font-family:"Calibri",sans-serif"><o:p></o:p></span></li></ol>
<p class="MsoPlainText"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif"><o:p> </o:p></span></p>
<p class="MsoPlainText"><b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">Concerns:</span></b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif"><o:p></o:p></span></p>
<ol style="margin-top:0in" start="1" type="1">
<li class="MsoPlainText" style="mso-list:l0 level1 lfo5"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">Because of the larger variety, email clients may offer better accessibility options than web browsers.</span><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">
<o:p></o:p></span></li></ol>
<p class="MsoPlainText"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif"><o:p> </o:p></span></p>
<p class="MsoPlainText"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif"><o:p> </o:p></span></p>
<p class="MsoPlainText"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">[1]
<a href="https://lists.llvm.org/pipermail/llvm-dev/2021-May/150344.html">https://lists.llvm.org/pipermail/llvm-dev/2021-May/150344.html</a><o:p></o:p></span></p>
<p class="MsoPlainText"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">[2]
<a href="https://llvm.org/docs/CodeReview.html#what-tools-are-used-for-code-review">
https://llvm.org/docs/CodeReview.html#what-tools-are-used-for-code-review</a><o:p></o:p></span></p>
<p class="MsoPlainText"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">[3]
<a href="https://lists.llvm.org/pipermail/llvm-dev/2021-April/150129.html">https://lists.llvm.org/pipermail/llvm-dev/2021-April/150129.html</a><o:p></o:p></span></p>
<p class="MsoPlainText"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">[4]
<a href="https://lists.llvm.org/pipermail/llvm-dev/2021-April/150136.html">https://lists.llvm.org/pipermail/llvm-dev/2021-April/150136.html</a><o:p></o:p></span></p>
<p class="MsoPlainText"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">[5]
<a href="https://lists.llvm.org/pipermail/llvm-dev/2021-April/150139.html">https://lists.llvm.org/pipermail/llvm-dev/2021-April/150139.html</a><o:p></o:p></span></p>
<p class="MsoPlainText"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif"><o:p> </o:p></span></p>
<p class="MsoPlainText"><o:p> </o:p></p>
<p class="MsoPlainText">-- <o:p></o:p></p>
<p class="MsoPlainText">Krzysztof Parzyszek <a href="mailto:kparzysz@quicinc.com">
kparzysz@quicinc.com</a> AI tools development<o:p></o:p></p>
<p class="MsoPlainText"><o:p> </o:p></p>
</div>
</body>
</html>