arsenm added inline comments.

================
Comment at: llvm/docs/CodeReview.rst:34
+uncertainty, a patch should be reviewed prior to being committed. If pre-commit
+code reviewes in a particular area have been requested, code should clear a
+significantly higher bar, e.g., fixes, to be commited without review.
----------------
jdoerfert wrote:
> hubert.reinterpretcast wrote:
> > If I understand this correctly, this is meant to cover situations where 
> > reviewers are active in an area and indicated an interest in reviewing 
> > basically everything. This seems redundant to likely-community-consensus. 
> > If someone is active in an area of the code, they would already know which 
> > reviewers are active and the area-specific culture. If someone is not 
> > active in an area of the code, the likely-community-consensus requirement 
> > should be enough to cause them to request a review.
> > 
> > Perhaps a clarification that "the likely-community-consensus requirement 
> > may vary depending on the specific area of the code" is enough. Or perhaps, 
> > "likely-community-consensus is not restricted to the content of the patch, 
> > but also includes the review process and culture".
> > If I understand this correctly, this is meant to cover situations where 
> > reviewers are active in an area and indicated an interest in reviewing 
> > basically everything. 
> 
> Pretty much, yes.
> 
> > This seems redundant to likely-community-consensus. If someone is active in 
> > an area of the code, they would already know which reviewers are active and 
> > the area-specific culture. If someone is not active in an area of the code, 
> > the likely-community-consensus requirement should be enough to cause them 
> > to request a review.
> 
> I would have agreed with you but this is in practice unfortunately not true.
> 
> > Perhaps a clarification that "the likely-community-consensus requirement 
> > may vary depending on the specific area of the code" is enough. Or perhaps, 
> > "likely-community-consensus is not restricted to the content of the patch, 
> > but also includes the review process and culture".
> 
> I'm fine with either. At the end of the day this is supposed to make it clear 
> that reviews cannot be circumvented while pointing to this rule if it is 
> clear reviews were requested.
Typo reviewes, and commited


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77683



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to