Thread (3 messages) 3 messages, 3 authors, 2019-12-28

Re: Updating the commit message for reverts

From: Gal Paikin <hidden>
Date: 2019-12-27 10:14:01

Hi,
Thanks for the reply!

So the idea of changing from "Revert Revert" to "Reland", "reapply"
has a big problem: sometimes Revert^2 actually means 'reverting
"Revert"' since "Revert" introduced a bug that wasn't in the original
change.

So to your question, I don't know what Revert^47 means since it
depends on each individual case. Sometimes it actually means "Revert"
and sometimes it means "Reland".

So do people actually use it? Yes! Many users reported to me that it
is not that unusual to get to "Revert^6", and it is very usual and
common to get to "Revert^2/3/4". It is also useful for the users to
know the number of the revert, according to the reports. Here is an
example:
https://android-review.googlesource.com/c/platform/art/+/352330
Feel free to also search for "Revert^2/3/4" to find many results.

Anyway, I am certain that "Revert^3" is better than "Revert revert
revert". There is definitely no clear way to solve this issue, but
perhaps "nth revert" would be a more "human language" solution?

Best,
Gal


On Tue, Dec 24, 2019 at 8:15 PM Junio C Hamano [off-list ref] wrote:
Gal Paikin [off-list ref] writes:
quoted
I work on the Gerrit team and I would like to change the default
behavior for suggested commit messages for Reverts.
Currently, if the user is trying a change called '"Revert "change X"',
the suggested commit message would be 'Revert "Revert "Change X""'
which is silly, since sometimes users want to revert the same change
many times.

The suggestion is to change the behavior to "Revert^N" instead of
multiple Reverts one after another.

I'm happy to change those things in Gerrit, but it would also be nice
if it were changed inside of Git.

What do you think?
I do not _think_ anybody lets the exact phrase 'Revert ...' in the
log message to be read by scripts to perform machannical action, so
in that sense it may be OK to special-case the reversion of a revert
and rephrase it in a more human friendly way.

BUT

 * what does "Revert^47" even mean?  Not just the proposed phrasing
   looks horrible, it is not even clear what happened at the end.
   Was the patch turned out to be OK after all these reversion war,
   or got rejected for now?  It also misleads readers who know Git
   can perform a merge with more than two parents that it may be
   reverting the effect relative to 47th parent of the commit.

   It _might_ be slightly more acceptable to flip the phrase between
   "Revert X" and "Reinstate X" (or "Reapply X"), without saying
   "this is the 47th round of our reversion war".  I dunno.

 * how often does it happen in practice?  If a group of developers
   find themselves reverting and reapplying the same commit more
   than a few times, wouldn't they rather stop and think before
   doing yet another round, which I expect to result in a better fix
   implemented as a separate and brand new patch that takes
   inspiration from a patch that was earlier reverted, and at that
   point it won't be the 47th iteration of reversion war anyway.

So, I am fairly negative on the change in the proposed form as-is.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help