[llvm-dev] [RFC] High-Level Code-Review Documentation Update

[Finkel, Hal J. via llvm-dev]

On 12/2/19 10:52 AM, Hubert Tong wrote:
On Thu, Nov 14, 2019 at 10:46 PM Finkel, Hal J. via llvm-dev <llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>> wrote:
  3. All comments by reviewers should be addressed by the patch author.
It is generally expected that suggested changes will be incorporated
into the next revision of the patch unless the author and/or other
reviewers can articulate a good reason to do otherwise (and then the
reviewers must agree).
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).


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.


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.


Good point. We should say "in a future revision"; there are a number of reasons why updates might be staged.

 -Hal



If you suggest changes in a code review, but
don't wish the suggestion to be interpreted this strongly, please state
so explicitly.

--
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20191203/c049973d/attachment.html>