[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] [RFC] Deprecate email code reviews in favor of Phabricator

[Martin Storsjö via lldb-dev]

On Mon, 3 May 2021, Krzysztof Parzyszek via cfe-dev wrote:


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


I don't mind formalizing that reviews are done on phabricator only. 
However following projects that way would, most probably, have one 
quite notable drawback compared with the current mailing list based 
approach:


Right now, it's easy to distinguish between mails requiring different 
levels of attention; ones with me in the To or CC fields are more visible 
and I try to read all of them. I have personal Herald rules that CC me on 
topics that I track. But I also browse the rest of the mails (quickly 
glancing usually only) for other topics I might be interested in.


My suspicion is that if the mail delivery is in the form of a personal 
subscription directly from Phabricator, it becomes much harder to 
distinguish mails that stem from just following a project as a whole, vs 
ones where I'm specifically CCd.


On the other hand I guess there can be other ways of filtering the mails 
do distinguish between those cases, so maybe it would be manageable?


// Martin
___
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

[Philip Reames via lldb-dev]

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 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 maili

[lldb-dev] Proposed Release Schedule for 12.0.1 and 13.0.0

[Tom Stellard via lldb-dev]

Hi,

Here is the proposed release schedule for the next 2 releases:

12.0.1

May 11: 12.0.1-rc1
June 8: 12.0.1-rc2
June 22: 12.0.1-final


13.0.0

July 27: release/13.x branch created
July 30: 13.0.0-rc1
Aug  24: 13.0.0-rc2
Sep   7: 13.0.0-rc3
Sep  21: 13.0.0-final

If there are no objections, I will add these to the website in a few days.

-Tom

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