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

[Philip Reames via lldb-dev]

Seems reasonable to me.  I'm not strongly in favor, but since I was 
strongly opposed to the previous proposal, a "don't object" seemed 
reasonable to share.

Philip

On 5/17/21 11:12 AM, Krzysztof Parzyszek via llvm-dev 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:*
>
>  1. 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.)
>  2. 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:*
>
>  1. 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.
>  2. Review authors and reviewers would only need to monitor one source
>     of comments without the fear that a review comment may end up
>     overlooked.
>  3. This changesimply codifies an existing practice.
>
> *Concerns:*
>
>  1. 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
>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20210517/1b997698/attachment.html>