[bug#48696] [PATCH 0/3] Documenting commit reverts and revocation

["Ludovic Courtès" <ludo@gnu.org>]

Hi Chris,

Christopher Baines <mail@cbaines.net> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:

[...]

>> I think you have a point though.  Could you propose different wording
>> for this section?
>>
>> (My goal for this section was to (1) spell out circumstances that may
>> lead to reverts, (2) explain the implications of committer
>> accountability, and (3) define our community standards in terms of
>> focusing on addressing issues and not on blaming individuals.)
>
> What I would like to see is more like this:
>
>   Problems happen, while minimising there occurrence is important, it's
>   also important to respond to problems in a useful way. There are two
>   priorities, mitigating the impact and understanding what happened in
>   order to reduce the chance of similar incidents in the future. The
>   responsibility for both these things primarily lies with those
>   involved, but like everything this is a group effort.
>
>   When working to mitigate the impact of a problem, obviously the
>   response is very much dependent on the situation. If it's possible to
>   fix things that are broken, that's preferable. If that's infeasible,
>   then promptly reverting changes to return to a working state is
>   justified (as with any commit, note why the change is being made in
>   the commit message).
>
>   Once the problem has been dealt with to some extent, then it's the
>   responsibility of those involved to make sure the situation is
>   understood. If you are working to understand what happened, focus on
>   gathering information and avoid assigning any blame. Do ask those
>   involved to describe what has happened, don't ask them to explain the
>   situation, even if you think they have something to explain, as this
>   implicitly blames them, which is unhelpful. Accountability comes from
>   a consensus about the problem, learning from it and improving
>   processes so that it's less likely to reoccur.
>
> I'm not sure how much needs saying about reverts, but I did include
> something.
>
> For committer accountability, that's where I'm talking about the
> "responsibilities of those involved". I guess that's a little vague, but
> what I'm trying to do there is trying to capture the group of relevant
> people, for example, the person who proposed the breaking change, the
> committer who pushed it, and the other person that reverted it.
>
> In terms of trying to focus on addressing issues and not blaming
> individuals, I think just avoiding language that implicitly blames
> people would be a big step forward. Whether that's enough, I'm unsure.

OK.

I like what you wrote; I think it addresses #3 and a bit of #2 above,
but I find a bit too abstract, not sufficiently hands-on (when can
commits be reverted? what’s the timeframe? who’s involved?), and lacking
examples.  “Problems happen” sounds unspecific to me (it reminds me of
Forest Gump :-)) and I’m uncomfortable with the passive voice that tends
to erase individuals.

  @subsection Addressing Issues
  
  Peer review (@pxref{Submitting Patches}) and tools such as
  @command{guix lint} (@pxref{Invoking guix lint}) and the test suite
  (@pxref{Running the Test Suite}) should catch issues before they are
  pushed.  Yet, commits that ``break'' functionality might occasionally
  go through.  When that happens, there are two priorities: mitigating
  the impact, and understanding what happened to reduce the chance of
  similar incidents in the future.  The responsibility for both these
  things primarily lies with those involved, but like everything this is
  a group effort.
  
  Some issues can directly affect all users---for instance because they
  make @command{guix pull} fail or break core functionality, because they
  break major packages (at build time or run time), or because they
  introduce known security vulnerabilities.
  
  @cindex reverting commits
  The people involved in authoring, reviewing, and pushing such
  commit(s) should be at the forefront to mitigate their impact in a
  timely fashion: by pushing a followup commit to fix it (if possible),
  or by reverting it to leave time to come up with a proper fix, and by
  communicating with other developers about the problem.
  
  If these persons are unavailable to address the issue in time, other
  committers are entitled to revert the commit(s), explaining in the
  commit log and on the mailing list what the problem was, with the goal
  of leaving time to the original committer, reviewer(s), and author(s)
  to propose a way forward.
  
  Once the problem has been dealt with, it is the responsibility of
  those involved to make sure the situation is understood.  If you are
  working to understand what happened, focus on gathering information
  and avoid assigning any blame.  Do ask those involved to describe what
  happened, do not ask them to explain the situation---this would
  implicitly blame them, which is unhelpful.  Accountability comes from
  a consensus about the problem, learning from it and improving
  processes so that it's less likely to reoccur.

There’s still “the people involved”, “these persons”, and “such commits”
(I removed “faulty”), because I couldn’t think of a way of avoiding
these without making the text too abstract or dismissing the idea
entirely (the idea that if I push a breaking change, others can expect
me to be spend time “mitigating the effort”).

Also, I think it’s useful to distinguish between “I revert my commit”
and “someone reverts my commit” due to their different social and
emotional implications (our goal is precisely to suggest that these
implications are out of place in this group effort that Guix is, but we
can’t deny that they preexist).

WDYT?

Thanks for taking the time to work on it!

Ludo’.