<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<p><br>
</p>
<div class="moz-cite-prefix">On 12/2/19 10:52 AM, Hubert Tong wrote:<br>
</div>
<blockquote type="cite" cite="mid:CACvkUqa18Gv5WsUQep7Yp8tgPFYNNx93gsGx=aQiwJ_eR7YqdA@mail.gmail.com">
<div dir="ltr">
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Thu, Nov 14, 2019 at 10:46 PM Finkel, Hal J. via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" moz-do-not-send="true">llvm-dev@lists.llvm.org</a>> wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
3. All comments by reviewers should be addressed by the patch author. <br>
It is generally expected that suggested changes will be incorporated <br>
into the next revision of the patch unless the author and/or other <br>
reviewers can articulate a good reason to do otherwise (and then the <br>
reviewers must agree).<br>
</blockquote>
<div>I disagree on the high bar here. The author should acknowledge the comments; however, addressing all of the comments in one shot has similar problems as having commits that are too large (diffs between revisions become more difficult to review).</div>
</div>
</div>
</blockquote>
<p><br>
</p>
<p>I'm happy to use acknowledge instead of address. That's probably closer to what I intended. It is certainly true that separable enhancements (i.e., not required for correctness) are normally better as follow up patches, and we should make sure that's clear.<br>
</p>
<p><br>
</p>
<blockquote type="cite" cite="mid:CACvkUqa18Gv5WsUQep7Yp8tgPFYNNx93gsGx=aQiwJ_eR7YqdA@mail.gmail.com">
<div dir="ltr">
<div class="gmail_quote">
<div>This also leads to significant timing issues, where the comments made overnight in some time zone are addressed by the author locally, but someone added comments in the afternoon the next day before the author has a chance to post the new revision.<br>
</div>
</div>
</div>
</blockquote>
<p><br>
</p>
<p>Good point. We should say "in a future revision"; there are a number of reasons why updates might be staged.</p>
<p> -Hal<br>
</p>
<p><br>
</p>
<blockquote type="cite" cite="mid:CACvkUqa18Gv5WsUQep7Yp8tgPFYNNx93gsGx=aQiwJ_eR7YqdA@mail.gmail.com">
<div dir="ltr">
<div class="gmail_quote">
<div> </div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
If you suggest changes in a code review, but <br>
don't wish the suggestion to be interpreted this strongly, please state <br>
so explicitly.<br>
</blockquote>
</div>
</div>
</blockquote>
<pre class="moz-signature" cols="72">--
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory</pre>
</body>
</html>