[PATCH] D71916: High-Level Code-Review Documentation Update

Aaron Ballman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 27 09:47:12 PST 2019


aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM!



================
Comment at: llvm/docs/CodeReview.rst:48
+
+Please note: There is no bar for post-commit feedback that is higher than for
+pre-commit feedback. Don't delay unnecessarily in providing feedback, but if
----------------
hfinkel wrote:
> MaskRay wrote:
> > The bar for post-commit feedback is not higher than pre-commit feedback ?
> Do you disagree? This is an important question. I've never felt that there is a higher bar, but I've certainly heard people say that they feel otherwise in practice. I don't think that's optimal - if someone has feedback, we should hear it, and they shouldn't feel barred by the fact that the commit has happened already.
> 
> Clearly there a practical timing issue: if I comment on a patch after the committer has stopped working on the project, I might not receive any reply, and I'll really just need to submit a patch myself. If there are additional distinctions that we wish to capture, I'm certainly happy to try to do so.
I think I read what @MaskRay wrote as a suggestion to improve the wording while keeping the same intent. The current phrasing is a bit awkward and I think the proposed wording is a bit better.

(FWIW, I agree that the bars are the same for pre- and post-commit feedback.)


================
Comment at: llvm/docs/CodeReview.rst:131
+certainly considered polite for non-trivial patches). Especially given the
+global nature of our community, this waiting time should be at least 24 hours.
+
----------------
Should we also mention something about keeping weekends and holidays in mind?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71916/new/

https://reviews.llvm.org/D71916





More information about the llvm-commits mailing list