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 kparz...@quicinc.com
<mailto:kparz...@quicinc.com> AI tools development
_______________________________________________
LLVM Developers mailing list
llvm-...@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
_______________________________________________
lldb-dev mailing list
lldb-dev@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
