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

[Krzysztof Parzyszek via lldb-dev]

Statement:

Our current code review policy states[1]:

"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 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:

"Code reviews are conducted on our web-based code-review tool (see Code Reviews 
with Phabricator)."



Current situation:

  1.  In a recent llvm-dev thread[2], 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.  I don't have specific information about post-commit reviews.  It seems 
like the most common form is an email reply to the auto-generated commit 
message, although (in my personal experience), "raising a concern" in the 
commit on Phabricator or commenting in the pre-commit review is usually 
sufficient to get author's attention.
  3.  We have Phabricator patches that automatically apply email comments to 
the Phabricator reviews, although reportedly this functionality is not fully 
reliable[3,4].  This can cause review comments to be lost in the email traffic.



Benefits:

  1.  Single way of doing code reviews: 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.  Local Phabricator extensions would no longer be needed.



Concerns:

  1.  For post-commit reviews, the commenter would need to find either the 
original review, or the Phabricator commit (e.g. 
https://reviews.llvm.org/rG06234f758e19).  Those are communicated (perhaps 
ironically) via email, which implies that those automatic emails should remain 
in place.
  2.  The current policy has been in place for a long time and it's expected 
that some people will continue using email for reviews out of habit or due to 
lack of awareness of the policy change.
  3.  Because of the larger variety, email clients may offer better 
accessibility options than web browsers.



Potential future direction:
This section presents a potential future evolution of the review process.  
Christian has conducted experiments suggesting that we can replace the 
XXX-commits mailing lists with notifications directly from Phabricator:

  *   For each of the mailing lists, we create a "project" with the same name 
in Phabricator, e.g. [5]. Every Phabricator user can join/leave these projects 
on their own.
  *   Everyone on these projects will receive the same email notifications from 
Phabricator as we have on the mailing lists. This is configured via "Herald" 
rules in Phabricator, as today, e.g. [7].
  *   Users can reply to these email notifications and Phabricator will 
incorporate these responses with their email client, see [6] for some example 
emails. Quoting and markup is supported as well.
  *   We do NOT migrate the membership lists. Users need to sign up to the 
projects manually. We will send an email with instructions to the mailing lists 
once everything is set up.
  *   The current XXX-commits mailing lists will be shut down
  *   The timeline for the migration is to be defined.
For experimenting, feel free to sign up to the prototype project at [5] . This 
project receives all commit and code review notifications.




[1] https://llvm.org/docs/CodeReview.html#what-tools-are-used-for-code-review

[2] https://lists.llvm.org/pipermail/llvm-dev/2021-April/150129.html

[3] https://lists.llvm.org/pipermail/llvm-dev/2021-April/150136.html

[4] https://lists.llvm.org/pipermail/llvm-dev/2021-April/150139.html
[5] https://reviews.llvm.org/project/view/104/
[6] https://reviews.llvm.org/D101432
[7] https://reviews.llvm.org/H769





--

Krzysztof Parzyszek  kparz...@quicinc.com   AI 
tools development


___
lldb-dev mailing list
lldb-dev@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

Re: [lldb-dev] [cfe-dev] [llvm-dev] [RFC] Deprecate email code reviews in favor of Phabricator

[Krzysztof Parzyszek via lldb-dev]

I'll defer to Christian the discussion about this section.

+Christian

-- 
Krzysztof Parzyszek  kparz...@quicinc.com   AI tools development

-Original Message-
From: cfe-dev  On Behalf Of Kevin P. Neal via 
cfe-dev
Sent: Monday, May 3, 2021 1:57 PM
To: Krzysztof Parzyszek via llvm-dev 
Cc: clangd-...@lists.llvm.org; openmp-...@lists.llvm.org; 
lldb-dev@lists.llvm.org; cfe-...@lists.llvm.org; libcxx-...@lists.llvm.org; 
flang-...@lists.llvm.org; parallel_libs-...@lists.llvm.org
Subject: [EXT] Re: [cfe-dev] [llvm-dev] [RFC] Deprecate email code reviews in 
favor of Phabricator

On Mon, May 03, 2021 at 05:24:24PM +, Krzysztof Parzyszek via llvm-dev 
wrote:
>This section presents a potential future evolution of the review
>process.  Christian has conducted experiments suggesting that we can
>replace the XXX-commits mailing lists with notifications directly from
>Phabricator:

Wouldn't this make it more difficult for sites that archive the lists?
Right now it all works. If the lists were eliminated then it would be harder to 
archive. Not impossible, but it would be more work.

Plus, how long would it take for archive sites to switch over? How much history 
would only exist in Phab's database?

Couldn't the commit lists be made read-only except from Phab? That would force 
reviews to happen on Phab but otherwise keep all existing email setups working.
--
"A method for inducing cats to exercise consists of directing a beam of 
invisible light produced by a hand-held laser apparatus onto the floor ...
in the vicinity of the cat, then moving the laser ... in an irregular way 
fascinating to cats,..." -- US patent 5443036, "Method of exercising a cat"
___
cfe-dev mailing list
cfe-...@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

Re: [lldb-dev] [llvm-dev] [RFC] Deprecate email code reviews in favor of Phabricator

[Krzysztof Parzyszek via lldb-dev]

I'm opposed to separating the pre- and post-commit reviews.  One of the goals 
of this proposal is to have the entire review history in one place, and using a 
combination of email and Phabricator would prevent that.  If I want to find out 
why a commit has been reverted, I have to search the post-commit emails to see 
what happened.  I guess one could argue that pre- and post-commit reviews could 
happen on different pages (Dxxx vs rGxxx), but, in my view, that is still 
better than emails.  The two concerns I have about post-commit reviews on 
Phabricator are that

  1.  People will keep doing it via email, because there is no mechanism (short 
of making the -commits lists read-only) that would actively inform them about 
the change in policy.
  2.  It takes a bit of manual work to get to the rG page for the commit, and 
if the commit message doesn't include the link to the pre-commit review, the 
corresponding D page may be hard to locate.

Regarding point (2), I'm hoping that something can be done in Phabricator to 
make the post-commit reviews easier: it may be as simple as sending the commit 
message to a list of subscribers.


--
Krzysztof Parzyszek  kparz...@quicinc.com   AI 
tools development

From: Philip Reames 
Sent: Monday, May 3, 2021 5:08 PM
To: Krzysztof Parzyszek ; llvm-dev 

Cc: clangd-...@lists.llvm.org; openmp-...@lists.llvm.org; 
lldb-dev@lists.llvm.org; cfe-...@lists.llvm.org; libcxx-...@lists.llvm.org; 
flang-...@lists.llvm.org; parallel_libs-...@lists.llvm.org
Subject: [EXT] Re: [llvm-dev] [RFC] Deprecate email code reviews in favor of 
Phabricator


In my view, this email is really too different topics.  Given that, my response 
is split into two parts.

First, should we make phabricator our default for code review?  I am not 
opposed to this.  I don't particular support it either, but I would not spend 
time arguing against it.  I would suggest that we re-frame the proposal to 
distinguish precommit and post commit review - with only the former moving to 
phabricator.  I have not seen post-commit done successfully on phabricator to 
date in any wide spread manner.

Second, should we consider retiring llvm-commits and the other mailing lists?  
My gut response is a flat out NO  What we have works.  I am highly 
reluctant to run the risk of breaking our existing processes - which for all 
their problems mostly work - for the, to me, seemingly very minimal value 
obtained by moving away from email discussion.  Post commit review in email 
works.  I strongly suspect that if you try to change that, you will either 
simply drive out post commit review discussion (bad idea!) or discussions will 
move to private email exchanges (bad idea!).  I'm open to being convinced here, 
but the burden of proof is high.  The risk we'd be talking about with such a 
transition is immense.

Philip
On 5/3/2021 10:24 AM, Krzysztof Parzyszek via llvm-dev wrote:



Statement:

Our current code review policy states[1]:

"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 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:

"Code reviews are conducted on our web-based code-review tool (see Code Reviews 
with Phabricator)."



Current situation:

  1.  In a recent llvm-dev thread[2], 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.  I don't have specific information about post-commit reviews.  It seems 
like the most common form is an email reply to the auto-generated commit 
message, although (in my personal experience), "raising a concern" in the 
commit on Phabricator or commenting in the pre-commit review is usually 
sufficient to get author's attention.
  3.  We have Phabricator patches that automatically apply email comments to 
the Phabricator reviews, although reportedly this functionality is not fully 
reliable[3,4].  This can cause review comments to be lost in the email traffic.



Benefits:

  1.  Single way of doing code reviews: 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.  Local Phabricator extensions would no longer be needed.



Concerns:

  1.  For post-commit reviews, the commenter would need to find either the 
or

Re: [lldb-dev] [cfe-dev] [RFC] Deprecate email code reviews in favor of Phabricator

[Krzysztof Parzyszek via lldb-dev]

You're right that doing post-commit reviews on Phabricator is not 
seamless---the rG link is not included anywhere.  Hopefully that could be fixed 
with some Phabricator configuration tweaks, like sending the commit email to 
the -commits list.

I'm not sure if there is a general fix for the spam issue.  I have had this 
problem as well with the existing lists, so it's not limited to Phabricator.  
In our spam filter I have whitelisted enough emails to avoid the unnecessary 
filtering almost completely, but it is an issue nevertheless.


-- 
Krzysztof Parzyszek  kparz...@quicinc.com   AI tools development

-Original Message-
From: Aaron Ballman  
Sent: Tuesday, May 4, 2021 6:24 AM
To: Krzysztof Parzyszek 
Cc: llvm-dev ; clangd-...@lists.llvm.org; 
openmp-...@lists.llvm.org; lldb-dev@lists.llvm.org; cfe-...@lists.llvm.org; 
libcxx-...@lists.llvm.org; flang-...@lists.llvm.org; 
parallel_libs-...@lists.llvm.org
Subject: [EXT] Re: [cfe-dev] [RFC] Deprecate email code reviews in favor of 
Phabricator

On Mon, May 3, 2021 at 1:24 PM Krzysztof Parzyszek via cfe-dev 
 wrote:
>
> Statement:
>
> Our current code review policy states[1]:
>
> “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 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:
>
> “Code reviews are conducted on our web-based code-review tool (see Code 
> Reviews with Phabricator).”

Personally, I am in favor of this policy for initiating code reviews, but am 
opposed to it for post-commit feedback. The problem, as I see it, is that not 
every change *gets* code review via Phab and the email lists are the only place 
on which to provide that post-commit feedback. This largely comes up in two 
ways: NFC changes and changes made by code owners in the area of the compiler 
which they own. We still need *some* mechanism by which to provide them 
post-commit feedback. Currently, the way we provide that is frequently via an 
email reply to the commit message on the *-commits list so that the issue can 
be seen by both the patch author and the community at large.

> Current situation:
>
> In a recent llvm-dev thread[2], 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.) I 
> don’t have specific information about post-commit reviews.  It seems like the 
> most common form is an email reply to the auto-generated commit message, 
> although (in my personal experience), “raising a concern” in the commit on 
> Phabricator or commenting in the pre-commit review is usually sufficient to 
> get author’s attention.
> We have Phabricator patches that automatically apply email comments to the 
> Phabricator reviews, although reportedly this functionality is not fully 
> reliable[3,4].  This can cause review comments to be lost in the email 
> traffic.
>
>
>
> Benefits:
>
> Single way of doing code reviews: 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.
> Local Phabricator extensions would no longer be needed.
>
>
>
> Concerns:
>
> For post-commit reviews, the commenter would need to find either the original 
> review, or the Phabricator commit (e.g. 
> https://reviews.llvm.org/rG06234f758e19).  Those are communicated (perhaps 
> ironically) via email, which implies that those automatic emails should 
> remain in place.

The Phab commit message does not have any subscribers though, and so my 
understanding is that comments on that Phab interface are not communicated out 
to the community as a whole, which means the community may miss out on 
important post-commit-review information like general awareness of the problem, 
workarounds people can use until the author corrects something, alternative 
ideas on how to fix the issue, etc. Or do I misunderstand the way Phab works in 
this workflow?

Also, "the commenter would need to find the Phabricator commit"
concerns me -- adding extra burden on the people providing feedback means that 
*some* amount of those people will struggle enough to simply not provide that 
feedback. Responding to an email is about as low as I think we can set that 
bar, so the current approach has better ergonomics for giving feedback. When I 
look at an existing NFC commit email 
(https:

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

[Krzysztof Parzyszek via lldb-dev]

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 change simply 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

[2] 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

[4] https://lists.llvm.org/pipermail/llvm-dev/2021-April/150136.html

[5] https://lists.llvm.org/pipermail/llvm-dev/2021-April/150139.html





--

Krzysztof Parzyszek  kparz...@quicinc.com   AI 
tools development


___
lldb-dev mailing list
lldb-dev@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

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

[Krzysztof Parzyszek via lldb-dev]

Post-commit reviews are conducted, in order of preference, on Phabricator,
This still seems like a change in practice that I'm not in favor of, personally 
- due to the current divergence between email and phab review feedback. Yes, 
this would be one way to unify it - but I'm not sure it's necessarily the best 
one.

I'd suggest leaving this to a separate proposal so as not to complicate/muddy 
the waters of the formalization of pre-commit review practice.

I simply broke up the existing sentence from the documentation into two parts, 
one about pre-commit reviews and the other about all other code reviews (which 
are basically the post-commit reviews, although I’m open to corrections here).  
The first part was modified to reflect the proposed change, the second part was 
left unchanged.  In this RFC I only want to change the part of the 
documentation that pertains specifically to pre-commit code reviews.  If the 
wording I used creates confusion, what would you suggest instead?


--
Krzysztof Parzyszek  kparz...@quicinc.com   AI 
tools development

From: David Blaikie 
Sent: Monday, May 17, 2021 4:40 PM
To: Krzysztof Parzyszek 
Cc: llvm-dev ; clangd-...@lists.llvm.org; 
openmp-...@lists.llvm.org; lldb-dev@lists.llvm.org; cfe-...@lists.llvm.org; 
libcxx-...@lists.llvm.org; flang-...@lists.llvm.org; 
parallel_libs-...@lists.llvm.org
Subject: [EXT] Re: [cfe-dev] [RFC] Deprecate pre-commit email code reviews in 
favor of Phabricator



On Mon, May 17, 2021 at 11:12 AM Krzysztof Parzyszek via cfe-dev 
mailto:cfe-...@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).
I'm with you here ^, this seems to document/formalize existing practice - 
though does this accurately reflect all the projects in the mororepo? I get the 
impression that mlir, maybe flang, etc might be doing reviews differently?

Post-commit reviews are conducted, in order of preference, on Phabricator,
This still seems like a change in practice that I'm not in favor of, personally 
- due to the current divergence between email and phab review feedback. Yes, 
this would be one way to unify it - but I'm not sure it's necessarily the best 
one.

I'd suggest leaving this to a separate proposal so as not to complicate/muddy 
the waters of the formalization of pre-commit review practice.

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 change simply 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

[2] 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

[4] https://lists.llvm.org/pipermail/llvm-dev/2021-April/150136.html

[5] https://lists.llvm.org/pipermail/llvm-dev/2021-April/150139.html





--

Krzysztof Parzyszek  kparz...@quicinc.com   AI 
tools development


___
cfe-dev mailing list
cfe-...@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
___
lldb-dev mailing list

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

[Krzysztof Parzyszek via lldb-dev]

Code review guidelines patch is available for review: 
https://reviews.llvm.org/D103811.


--
Krzysztof Parzyszek  kparz...@quicinc.com   AI 
tools development

From: David Blaikie 
Sent: Tuesday, May 18, 2021 3:01 PM
To: Krzysztof Parzyszek 
Cc: llvm-dev ; clangd-...@lists.llvm.org; 
openmp-...@lists.llvm.org; lldb-dev@lists.llvm.org; cfe-...@lists.llvm.org; 
libcxx-...@lists.llvm.org; flang-...@lists.llvm.org; 
parallel_libs-...@lists.llvm.org
Subject: [EXT] Re: [cfe-dev] [RFC] Deprecate pre-commit email code reviews in 
favor of Phabricator

On Tue, May 18, 2021 at 6:50 AM Krzysztof Parzyszek 
mailto:kparz...@quicinc.com>> wrote:

Post-commit reviews are conducted, in order of preference, on Phabricator,
This still seems like a change in practice that I'm not in favor of, personally 
- due to the current divergence between email and phab review feedback. Yes, 
this would be one way to unify it - but I'm not sure it's necessarily the best 
one.

I'd suggest leaving this to a separate proposal so as not to complicate/muddy 
the waters of the formalization of pre-commit review practice.

I simply broke up the existing sentence from the documentation into two parts, 
one about pre-commit reviews and the other about all other code reviews (which 
are basically the post-commit reviews, although I’m open to corrections here).  
The first part was modified to reflect the proposed change, the second part was 
left unchanged.

I think the issue is that the original phrasing was probably only intended to 
describe the preference for pre-commit review. (I think statements about 
post-commit review could reasonably read to be only those that say "post-commit 
review", in this ( 
https://llvm.org/docs/CodeReview.html#can-code-be-reviewed-after-it-is-committed
 ) section.

So I think (at least in terms of how to read it in a way that matches existing 
practice) the original wording amounted to something like this:

... "post-commit review can use any of the tools listed below" ...
... "pre-commit review is done in this order of phab, email, etc... "

ie: the post-commit review didn't have the same order of preference as 
pre-commit review.

I'd probably pull out the post-commit review-specific wording back up to where 
post-commit review is discussed, and leave the rest of this to talk about 
pre-commit review (most of this document discussing unqualified "review" seems 
predominantly to be talking about "pre-commit review" except the part that 
talks about "post commit review").

Probably move the "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." (without the "in order of 
preference") up to the "post-commit review" section, instead of referencing a 
version of it here.

In this RFC I only want to change the part of the documentation that pertains 
specifically to pre-commit code reviews.  If the wording I used creates 
confusion, what would you suggest instead?


--
Krzysztof Parzyszek  kparz...@quicinc.com   AI 
tools development

From: David Blaikie mailto:dblai...@gmail.com>>
Sent: Monday, May 17, 2021 4:40 PM
To: Krzysztof Parzyszek mailto:kparz...@quicinc.com>>
Cc: llvm-dev mailto:llvm-...@lists.llvm.org>>; 
clangd-...@lists.llvm.org; 
openmp-...@lists.llvm.org; 
lldb-dev@lists.llvm.org; 
cfe-...@lists.llvm.org; 
libcxx-...@lists.llvm.org; 
flang-...@lists.llvm.org; 
parallel_libs-...@lists.llvm.org
Subject: [EXT] Re: [cfe-dev] [RFC] Deprecate pre-commit email code reviews in 
favor of Phabricator



On Mon, May 17, 2021 at 11:12 AM Krzysztof Parzyszek via cfe-dev 
mailto:cfe-...@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).
I'm with you here ^, this seems to document/formalize existing practice - 
though does this accurately reflect all the projects in the mororepo? I get the 
impression that mlir, maybe flang, etc might be doing reviews differently?

Post-commit reviews are conducted, in order of preference, on Phab