[cfe-dev] [RFC] Deprecate pre-commit email code reviews in favor of Phabricator

[Chris Lattner via cfe-dev]

+1, makes sense to me.

-Chris

> On May 17, 2021, at 11:12 AM, Krzysztof Parzyszek via cfe-dev <cfe-dev at lists.llvm.org> wrote:
> 
> This is a revision of the previous RFC[1].  This RFC limits the scope to pre-commit reviews only.
>  
> Statement:
> Our current code review policy states[2]:
> “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.”
> 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:
> “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.”
>  
> Current situation:
> 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.)
> 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.
>  
> Benefits:
> 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.
> Review authors and reviewers would only need to monitor one source of comments without the fear that a review comment may end up overlooked.
> This change simply codifies an existing practice.
>  
> Concerns:
> Because of the larger variety, email clients may offer better accessibility options than web browsers.
>  
>  
> [1] https://lists.llvm.org/pipermail/llvm-dev/2021-May/150344.html <https://lists.llvm.org/pipermail/llvm-dev/2021-May/150344.html>
> [2] 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>
> [3] https://lists.llvm.org/pipermail/llvm-dev/2021-April/150129.html <https://lists.llvm.org/pipermail/llvm-dev/2021-April/150129.html>
> [4] https://lists.llvm.org/pipermail/llvm-dev/2021-April/150136.html <https://lists.llvm.org/pipermail/llvm-dev/2021-April/150136.html>
> [5] https://lists.llvm.org/pipermail/llvm-dev/2021-April/150139.html <https://lists.llvm.org/pipermail/llvm-dev/2021-April/150139.html>
>  
>  
> -- 
> Krzysztof Parzyszek  kparzysz at quicinc.com <mailto:kparzysz at quicinc.com>   AI tools development
>  
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev <https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20210518/d235cada/attachment.html>