llvm dev - Mar 2021 - Documenting community norms around reverts

I've posted a patch <https://reviews.llvm.org/D99305> to document our 
norms around reverts.  I was recently surprised to not find anything in 
this spirit.  A lot of newcomers find our norms a bit odd, so having 
this explicitly spelled out seems very worthwhile.

I tried to stick to stuff that is uncontroversial in the community. If 
you see something here that less than 90+% of the community would agree 
with, please point it out. I'll remove it for now, and possibly return 
to individual points if warranted in future reviews.

Philip

p.s. Draft text follows, but please make detailed wording suggestions on 
the review.

Reverts
-------

As a community, we strongly value having the tip of tree in a good 
state.  As
such, we tend to make much heaver use of reverts than some other open source
projects, and our norms are a bit different.

When should you revert your own change?

* Any time you learn of a serious problem with a change, you should 
revert it.
   We strongly encourage "revert to green" as opposed to "fixing 
forward".  We
   encourage reverting first, investigating offline, and then reapplying the
   fixed patch - possibly after another round of review if warranted.
* If you break a buildbot in a way which can't be quickly fixed, please 
revert.
* If a test case is reported in the commit thread which demonstrates a 
problem
   please revert, and investigate offline.
* If you are asked to revert by another contributor, please revert and 
discuss
   the merits of the request offline (unless doing so would further 
destabilize
   trip of tree).

When should you revert someone else's change?

* In general, if the author themselves would revert the change per these
   guidelines, we encourage other contributors to do so as a favor to the
   author.  This is one of the major cases where our norms differ from 
others;
   we generally consider reverting someones change to be a favor to 
them.  We
   don't expect contributors to be always available, and the assurance 
that a
   problematic patch will be reverted and we can return to it at our next
   opportunity enables this.
* The other case for a third-party revert is for serious norm 
violations.  As a
   general rule, you should never be reverting a patch for this unless 
you've
   already started a discussion highlight the problem on the original commit
   thread.  There are exceptions where a rapid revert for a norm 
violation may
   be called for, but if those are relevant for you, you already know.

What are the expectations around a revert?

* You should be sure that reverting the change improves the stability of tip
   of tree.  Sometimes reverting one change in a series can worsen things
   instead of improving them.  We expect reasonable judgment to ensure that
   the proper patch or set of patches is being reverted.
* You should have a publicly reproducible test case ready to share.  It is
   not considered reasonable to revert without at least the promise to 
produce
   a public test case in the near term.  We encourage sharing of test 
cases in
   commit threads, or in PRs.  We encourage the reverter to minimize the 
test
   case and to prune dependencies where practical.
* Reverts should be reasonably timely.  A change submitted two hours ago
   can be reverted by a third-party without prior discussion.  A change
   submitted two years ago probably shouldn't be.  Where exactly the 
transition
   point is is hard to say, but it's probably in the handful of days in tree
   territory.    If you are unsure, we encourage you to reply to the commit
   thread, give the author a bit to respond, and then proceed with the 
revert
   if the author doesn't seem to be actively responding.

How should you respond if someone reverted your change?

* In general, the appropriate response is to start by thanking them.  If you
   need more information to address the problem, please follow up in the
   original commit thread with the reverting patch author.
* It is normal and healthy to have patches reverted.  Having a patch 
reverted
   does not neccessarily mean you did anything wrong.

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20210324/f09e0171/attachment.html>

El mié., 24 de mar. de 2021 4:53 PM, Philip Reames via llvm-dev <
llvm-dev at lists.llvm.org> escribió:
> I've posted a patch <https://reviews.llvm.org/D99305> to document
our
> norms around reverts.  I was recently surprised to not find anything in
> this spirit.  A lot of newcomers find our norms a bit odd, so having this
> explicitly spelled out seems very worthwhile.
>
> I tried to stick to stuff that is uncontroversial in the community. If you
> see something here that less than 90+% of the community would agree with,
> please point it out. I'll remove it for now, and possibly return to
> individual points if warranted in future reviews.
>
> Philip
>
> p.s. Draft text follows, but please make detailed wording suggestions on
> the review.
>
> Reverts
> -------
>
> As a community, we strongly value having the tip of tree in a good state.
> As
> such, we tend to make much heaver use of reverts than some other open
> source
> projects, and our norms are a bit different.
>
> When should you revert your own change?
>
> * Any time you learn of a serious problem with a change, you should revert
> it.
>   We strongly encourage "revert to green" as opposed to
"fixing forward".
> We
>   encourage reverting first, investigating offline, and then reapplying the
>   fixed patch - possibly after another round of review if warranted.
> * If you break a buildbot in a way which can't be quickly fixed, please
> revert.
> * If a test case is reported in the commit thread which demonstrates a
> problem
>   please revert, and investigate offline.
> * If you are asked to revert by another contributor, please revert and
> discuss
>   the merits of the request offline (unless doing so would further
> destabilize
>   trip of tree).
>
> When should you revert someone else's change?
>
> * In general, if the author themselves would revert the change per these
>   guidelines, we encourage other contributors to do so as a favor to the
>   author.  This is one of the major cases where our norms differ from
> others;
>   we generally consider reverting someones change to be a favor to them.
> We
>   don't expect contributors to be always available, and the assurance
that
> a
>   problematic patch will be reverted and we can return to it at our next
>   opportunity enables this.
> * The other case for a third-party revert is for serious norm violations.
> As a
>   general rule, you should never be reverting a patch for this unless
> you've
>   already started a discussion highlight the problem on the original commit
>   thread.  There are exceptions where a rapid revert for a norm violation
> may
>   be called for, but if those are relevant for you, you already know.
>
> What are the expectations around a revert?
>
> * You should be sure that reverting the change improves the stability of
> tip
>   of tree.  Sometimes reverting one change in a series can worsen things
>   instead of improving them.  We expect reasonable judgment to ensure that
>   the proper patch or set of patches is being reverted.
> * You should have a publicly reproducible test case ready to share.  It is
>   not considered reasonable to revert without at least the promise to
> produce
>   a public test case in the near term.  We encourage sharing of test cases
> in
>   commit threads, or in PRs.  We encourage the reverter to minimize the
> test
>   case and to prune dependencies where practical.
> * Reverts should be reasonably timely.  A change submitted two hours ago
>   can be reverted by a third-party without prior discussion.  A change
>   submitted two years ago probably shouldn't be.  Where exactly the
> transition
>   point is is hard to say, but it's probably in the handful of days in
tree
>   territory.    If you are unsure, we encourage you to reply to the commit
>   thread, give the author a bit to respond, and then proceed with the
> revert
>   if the author doesn't seem to be actively responding.
>
> How should you respond if someone reverted your change?
>
> * In general, the appropriate response is to start by thanking them.  If
> you
>   need more information to address the problem, please follow up in the
>   original commit thread with the reverting patch author.
> * It is normal and healthy to have patches reverted.  Having a patch
> reverted
>   does not neccessarily mean you did anything wrong.
>
> _______________________________________________
> 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/llvm-dev/attachments/20210324/f94ece53/attachment.html>

Thanks for writing this up, I really like this text. Somehow I feel like it
captures a collaborative spirit.

On Wed, Mar 24, 2021 at 3:53 PM Philip Reames via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> I've posted a patch <https://reviews.llvm.org/D99305> to document
our
> norms around reverts.  I was recently surprised to not find anything in
> this spirit.  A lot of newcomers find our norms a bit odd, so having this
> explicitly spelled out seems very worthwhile.
>
> I tried to stick to stuff that is uncontroversial in the community. If you
> see something here that less than 90+% of the community would agree with,
> please point it out. I'll remove it for now, and possibly return to
> individual points if warranted in future reviews.
>
> Philip
>
> p.s. Draft text follows, but please make detailed wording suggestions on
> the review.
>
> Reverts
> -------
>
> As a community, we strongly value having the tip of tree in a good state.
> As
> such, we tend to make much heaver use of reverts than some other open
> source
> projects, and our norms are a bit different.
>
> When should you revert your own change?
>
> * Any time you learn of a serious problem with a change, you should revert
> it.
>   We strongly encourage "revert to green" as opposed to
"fixing forward".
> We
>   encourage reverting first, investigating offline, and then reapplying the
>   fixed patch - possibly after another round of review if warranted.
> * If you break a buildbot in a way which can't be quickly fixed, please
> revert.
> * If a test case is reported in the commit thread which demonstrates a
> problem
>   please revert, and investigate offline.
> * If you are asked to revert by another contributor, please revert and
> discuss
>   the merits of the request offline (unless doing so would further
> destabilize
>   trip of tree).
>
> When should you revert someone else's change?
>
> * In general, if the author themselves would revert the change per these
>   guidelines, we encourage other contributors to do so as a favor to the
>   author.  This is one of the major cases where our norms differ from
> others;
>   we generally consider reverting someones change to be a favor to them.
> We
>   don't expect contributors to be always available, and the assurance
that
> a
>   problematic patch will be reverted and we can return to it at our next
>   opportunity enables this.
> * The other case for a third-party revert is for serious norm violations.
> As a
>   general rule, you should never be reverting a patch for this unless
> you've
>   already started a discussion highlight the problem on the original commit
>   thread.  There are exceptions where a rapid revert for a norm violation
> may
>   be called for, but if those are relevant for you, you already know.
>
> What are the expectations around a revert?
>
> * You should be sure that reverting the change improves the stability of
> tip
>   of tree.  Sometimes reverting one change in a series can worsen things
>   instead of improving them.  We expect reasonable judgment to ensure that
>   the proper patch or set of patches is being reverted.
> * You should have a publicly reproducible test case ready to share.  It is
>   not considered reasonable to revert without at least the promise to
> produce
>   a public test case in the near term.  We encourage sharing of test cases
> in
>   commit threads, or in PRs.  We encourage the reverter to minimize the
> test
>   case and to prune dependencies where practical.
> * Reverts should be reasonably timely.  A change submitted two hours ago
>   can be reverted by a third-party without prior discussion.  A change
>   submitted two years ago probably shouldn't be.  Where exactly the
> transition
>   point is is hard to say, but it's probably in the handful of days in
tree
>   territory.    If you are unsure, we encourage you to reply to the commit
>   thread, give the author a bit to respond, and then proceed with the
> revert
>   if the author doesn't seem to be actively responding.
>
> How should you respond if someone reverted your change?
>
> * In general, the appropriate response is to start by thanking them.  If
> you
>   need more information to address the problem, please follow up in the
>   original commit thread with the reverting patch author.
> * It is normal and healthy to have patches reverted.  Having a patch
> reverted
>   does not neccessarily mean you did anything wrong.
>
> _______________________________________________
> 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/llvm-dev/attachments/20210325/0f5a6be1/attachment.html>

On Wed, Mar 24, 2021 at 03:52:51PM -0700, Philip Reames via llvm-dev
wrote:
> I tried to stick to stuff that is uncontroversial in the community. If you
> see something here that less than 90+% of the community would agree with,
> please point it out. I'll remove it for now, and possibly return to
> individual points if warranted in future reviews.
I'd like to see some wording to make sure flagging by buildbot is
actually somewhat logically related to that commit. We've had enough
issues with both flacky bots and the general 20 pages blame list that I
believe it is worth mentioning.

Joerg