{"thread":{"id":"52514","subject":"Updating the commit message for reverts","startedAt":"2019-12-24T11:07:05Z","lastAt":"2019-12-30T20:41:26Z","messageCount":8,"participants":["Gal Paikin","Junio C Hamano","Danh Doan","Jonathan Nieder","Oswald Buddenhagen"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"388864","messageId":"CAEsQYpNtgMgwjVSOYB9vn-YPvKyKPZ2yZ3NigAVe3PztTN4v8w@mail.gmail.com","threadId":"52514","inReplyTo":null,"subject":"Updating the commit message for reverts","fromName":"Gal Paikin","fromEmail":"paiking@google.com","sentAt":"2019-12-24T11:06:51Z","receivedAt":"2019-12-24T11:07:05Z","isPatch":false,"sender":{"key":"paiking@google.com","avatar":null},"body":"Hi,\n\nI work on the Gerrit team and I would like to change the default\nbehavior for suggested commit messages for Reverts.\nCurrently, if the user is trying a change called '\"Revert \"change X\"',\nthe suggested commit message would be 'Revert \"Revert \"Change X\"\"'\nwhich is silly, since sometimes users want to revert the same change\nmany times.\n\nThe suggestion is to change the behavior to \"Revert^N\" instead of\nmultiple Reverts one after another.\n\nI'm happy to change those things in Gerrit, but it would also be nice\nif it were changed inside of Git.\n\nWhat do you think?\n\nThanks!\nGal Paikin\n"},{"id":"388872","messageId":"xmqq7e2l5ych.fsf@gitster-ct.c.googlers.com","threadId":"52514","inReplyTo":"CAEsQYpNtgMgwjVSOYB9vn-YPvKyKPZ2yZ3NigAVe3PztTN4v8w@mail.gmail.com","subject":"Re: Updating the commit message for reverts","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-12-24T19:15:42Z","receivedAt":"2019-12-24T19:16:02Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Gal Paikin writes:\n\n> I work on the Gerrit team and I would like to change the default\n> behavior for suggested commit messages for Reverts.\n> Currently, if the user is trying a change called '\"Revert \"change X\"',\n> the suggested commit message would be 'Revert \"Revert \"Change X\"\"'\n> which is silly, since sometimes users want to revert the same change\n> many times.\n>\n> The suggestion is to change the behavior to \"Revert^N\" instead of\n> multiple Reverts one after another.\n>\n> I'm happy to change those things in Gerrit, but it would also be nice\n> if it were changed inside of Git.\n>\n> What do you think?\n\nI do not _think_ anybody lets the exact phrase 'Revert ...' in the\nlog message to be read by scripts to perform machannical action, so\nin that sense it may be OK to special-case the reversion of a revert\nand rephrase it in a more human friendly way.\n\nBUT\n\n * what does \"Revert^47\" even mean? Not just the proposed phrasing\n looks horrible, it is not even clear what happened at the end.\n Was the patch turned out to be OK after all these reversion war,\n or got rejected for now? It also misleads readers who know Git\n can perform a merge with more than two parents that it may be\n reverting the effect relative to 47th parent of the commit.\n\n It _might_ be slightly more acceptable to flip the phrase between\n \"Revert X\" and \"Reinstate X\" (or \"Reapply X\"), without saying\n \"this is the 47th round of our reversion war\". I dunno.\n\n * how often does it happen in practice? If a group of developers\n find themselves reverting and reapplying the same commit more\n than a few times, wouldn't they rather stop and think before\n doing yet another round, which I expect to result in a better fix\n implemented as a separate and brand new patch that takes\n inspiration from a patch that was earlier reverted, and at that\n point it won't be the 47th iteration of reversion war anyway.\n\nSo, I am fairly negative on the change in the proposed form as-is.\n"},{"id":"388965","messageId":"CAEsQYpMJGbw3L66vCd25Ht0bTBzvvt1yMRd2U3=u3U-BZukyzg@mail.gmail.com","threadId":"52514","inReplyTo":"xmqq7e2l5ych.fsf@gitster-ct.c.googlers.com","subject":"Re: Updating the commit message for reverts","fromName":"Gal Paikin","fromEmail":"paiking@google.com","sentAt":"2019-12-27T10:13:47Z","receivedAt":"2019-12-27T10:14:01Z","isPatch":false,"sender":{"key":"paiking@google.com","avatar":null},"body":"Hi,\nThanks for the reply!\n\nSo the idea of changing from \"Revert Revert\" to \"Reland\", \"reapply\"\nhas a big problem: sometimes Revert^2 actually means 'reverting\n\"Revert\"' since \"Revert\" introduced a bug that wasn't in the original\nchange.\n\nSo to your question, I don't know what Revert^47 means since it\ndepends on each individual case. Sometimes it actually means \"Revert\"\nand sometimes it means \"Reland\".\n\nSo do people actually use it? Yes! Many users reported to me that it\nis not that unusual to get to \"Revert^6\", and it is very usual and\ncommon to get to \"Revert^2/3/4\". It is also useful for the users to\nknow the number of the revert, according to the reports. Here is an\nexample:\nhttps://android-review.googlesource.com/c/platform/art/+/352330\nFeel free to also search for \"Revert^2/3/4\" to find many results.\n\nAnyway, I am certain that \"Revert^3\" is better than \"Revert revert\nrevert\". There is definitely no clear way to solve this issue, but\nperhaps \"nth revert\" would be a more \"human language\" solution?\n\nBest,\nGal\n\n\nOn Tue, Dec 24, 2019 at 8:15 PM Junio C Hamano wrote:\n>\n> Gal Paikin writes:\n>\n> > I work on the Gerrit team and I would like to change the default\n> > behavior for suggested commit messages for Reverts.\n> > Currently, if the user is trying a change called '\"Revert \"change X\"',\n> > the suggested commit message would be 'Revert \"Revert \"Change X\"\"'\n> > which is silly, since sometimes users want to revert the same change\n> > many times.\n> >\n> > The suggestion is to change the behavior to \"Revert^N\" instead of\n> > multiple Reverts one after another.\n> >\n> > I'm happy to change those things in Gerrit, but it would also be nice\n> > if it were changed inside of Git.\n> >\n> > What do you think?\n>\n> I do not _think_ anybody lets the exact phrase 'Revert ...' in the\n> log message to be read by scripts to perform machannical action, so\n> in that sense it may be OK to special-case the reversion of a revert\n> and rephrase it in a more human friendly way.\n>\n> BUT\n>\n> * what does \"Revert^47\" even mean? Not just the proposed phrasing\n> looks horrible, it is not even clear what happened at the end.\n> Was the patch turned out to be OK after all these reversion war,\n> or got rejected for now? It also misleads readers who know Git\n> can perform a merge with more than two parents that it may be\n> reverting the effect relative to 47th parent of the commit.\n>\n> It _might_ be slightly more acceptable to flip the phrase between\n> \"Revert X\" and \"Reinstate X\" (or \"Reapply X\"), without saying\n> \"this is the 47th round of our reversion war\". I dunno.\n>\n> * how often does it happen in practice? If a group of developers\n> find themselves reverting and reapplying the same commit more\n> than a few times, wouldn't they rather stop and think before\n> doing yet another round, which I expect to result in a better fix\n> implemented as a separate and brand new patch that takes\n> inspiration from a patch that was earlier reverted, and at that\n> point it won't be the 47th iteration of reversion war anyway.\n>\n> So, I am fairly negative on the change in the proposed form as-is.\n"},{"id":"389022","messageId":"20191228132022.GD24268@danh.dev","threadId":"52514","inReplyTo":"CAEsQYpMJGbw3L66vCd25Ht0bTBzvvt1yMRd2U3=u3U-BZukyzg@mail.gmail.com","subject":"Re: Updating the commit message for reverts","fromName":"Danh Doan","fromEmail":"congdanhqx@gmail.com","sentAt":"2019-12-28T13:20:22Z","receivedAt":"2019-12-28T13:25:20Z","isPatch":false,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"On 2019-12-27 11:13:47+0100, Gal Paikin wrote:\n> Hi,\n> Thanks for the reply!\n> \n> So the idea of changing from \"Revert Revert\" to \"Reland\", \"reapply\"\n> has a big problem: sometimes Revert^2 actually means 'reverting\n> \"Revert\"' since \"Revert\" introduced a bug that wasn't in the original\n> change.\n> \n> So to your question, I don't know what Revert^47 means since it\n> depends on each individual case. Sometimes it actually means \"Revert\"\n> and sometimes it means \"Reland\".\n> \n> So do people actually use it? Yes! Many users reported to me that it\n> is not that unusual to get to \"Revert^6\", and it is very usual and\n\nI've seen Revert x6 in a code base, I couldn't get to know the reason\nfor that reversion war. I think it could be seen more in some in-house\nweb development that uses trunk-based development, code is being\ntested with CI/CD, lightly tested, squash-merged to master,\nthen run into problem in staging (or worst, production, because not\nenough traffix was generated for testing environment).\n\n> common to get to \"Revert^2/3/4\". It is also useful for the users to\n> know the number of the revert, according to the reports. Here is an\n> example:\n> https://android-review.googlesource.com/c/platform/art/+/352330\n> Feel free to also search for \"Revert^2/3/4\" to find many results.\n> \n> Anyway, I am certain that \"Revert^3\" is better than \"Revert revert\n> revert\". There is definitely no clear way to solve this issue, but\n> perhaps \"nth revert\" would be a more \"human language\" solution?\n\nIn my very personal opinion, \"nth revert\" is a poor choice.\nAt a first glance, I would take it as:\n\n\tThis is the \"nth revert\", after applying this patch n times.\n\n-- \nDanh\n"},{"id":"389075","messageId":"xmqqwoadhi2m.fsf@gitster-ct.c.googlers.com","threadId":"52514","inReplyTo":"CAEsQYpMJGbw3L66vCd25Ht0bTBzvvt1yMRd2U3=u3U-BZukyzg@mail.gmail.com","subject":"Re: Updating the commit message for reverts","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-12-30T16:52:17Z","receivedAt":"2019-12-30T16:52:27Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Gal Paikin writes:\n\n> So the idea of changing from \"Revert Revert\" to \"Reland\", \"reapply\"\n> has a big problem: sometimes Revert^2 actually means 'reverting\n> \"Revert\"' since \"Revert\" introduced a bug that wasn't in the original\n> change.\n\nSorry, I do not see a relevance of the above in this discussion, as\nthe situation does not improve if you phrase it as \"Revert^2\" or\n\"Second Revert\".\n\nAlso as somebody else said in downthread, the phrasing \"second\nrevert\" would typically mean \"a patch gets applied, proves to be\npremature and gets reverted, the revert is reverted because the\nsituation in the rest of the system improved to make the orignal\npatch viable, and then gets reverted again due to some other\nissues\", i.e. \"Revert Revert Revert do something\", so it is even\nworse.\n\n"},{"id":"389097","messageId":"20191230195505.GD57251@google.com","threadId":"52514","inReplyTo":"CAEsQYpNtgMgwjVSOYB9vn-YPvKyKPZ2yZ3NigAVe3PztTN4v8w@mail.gmail.com","subject":"Re: Updating the commit message for reverts","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2019-12-30T19:55:05Z","receivedAt":"2019-12-30T19:55:09Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"(also cc-ing the Gerrit list. The git mailing list requires plain-text\n email and prefers bottom-posting; there shouldn't be any other\n gotchas if you reply-all)\nHi Gal,\n\nGal Paikin wrote:\n\n> I work on the Gerrit team and I would like to change the default\n> behavior for suggested commit messages for Reverts.\n\nThanks for writing. I like it when tools behave consistently with one\nanother, so I'm glad you brought it up here.\n\n> Currently, if the user is trying a change called '\"Revert \"change X\"',\n> the suggested commit message would be 'Revert \"Revert \"Change X\"\"'\n> which is silly, since sometimes users want to revert the same change\n> many times.\n\nFor context, [1] is where this was discussed previously on the Gerrit\nmailing list. Do you have more context? For example, have there been\nreports from users requesting this kind of change?\n\nFor a bit of precedent, the Chromium project has some custom logic[2]\nthat rewrites a revert of a revert to \"Reland\". I kind of like that,\nsince it makes the intention behind the revert-of-revert a little\nclearer.\n\nIf both reverts are clean, then the typical use case I can think of is\nthat a prerequisite was missing, hence the revert; after all the\nprerequisites were in place, it was then safe to roll forward, hence\nthe reland. I would hope that the full commit messages for these\nrevert commits would provide this context since without that kind of\ncommunication from the people involved, a reader is lost.\n\n> The suggestion is to change the behavior to \"Revert^N\" instead of\n> multiple Reverts one after another.\n\nThis would be replacing one kind of jargon with another, so it's not\nclear to me that it would improve matters.\n\nWith Revert / Reland, we can forget the N altogether:\n\n 1. 'Do some great thing'\n 2. 'Revert \"Do some great thing\"'\n 3. 'Reland \"Do some great thing\"'\n 4. 'Revert \"Do some great thing\"'\n 5. (etc)\n\nFor the reader of the shortlog, it's not too important how long the\nedit war has gone. The single word makes it clear what the commit\nis going to do.\n\nIn the full commit message, I would hope that people put in some\nexplanation of the story so far to explain why they have gone\nback-and-forth all these times.\n\nThanks and hope that helps,\nJonathan\n\n[1] https://groups.google.com/d/msg/repo-discuss/edl43LcyEKE/f9Q65kOcBgAJ\n[2] https://chromium-review.googlesource.com/c/infra/gerrit-plugins/chromium-behavior/+/757684\n"},{"id":"389098","messageId":"20191230195915.GE57251@google.com","threadId":"52514","inReplyTo":"CAEsQYpNtgMgwjVSOYB9vn-YPvKyKPZ2yZ3NigAVe3PztTN4v8w@mail.gmail.com","subject":"Re: Updating the commit message for reverts","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2019-12-30T19:59:15Z","receivedAt":"2019-12-30T19:59:19Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"(not cc-ing the Gerrit list after all, since only members can post)\nHi Gal,\n\nGal Paikin wrote:\n\n> I work on the Gerrit team and I would like to change the default\n> behavior for suggested commit messages for Reverts.\n\nThanks for writing. I like it when tools behave consistently with one\nanother, so I'm glad you brought it up here.\n\n> Currently, if the user is trying a change called '\"Revert \"change X\"',\n> the suggested commit message would be 'Revert \"Revert \"Change X\"\"'\n> which is silly, since sometimes users want to revert the same change\n> many times.\n\nFor context, [1] is where this was discussed previously on the Gerrit\nmailing list. Do you have more context? For example, have there been\nreports from users requesting this kind of change?\n\nFor a bit of precedent, the Chromium project has some custom logic[2]\nthat rewrites a revert of a revert to \"Reland\". I kind of like that,\nsince it makes the intention behind the revert-of-revert a little\nclearer.\n\nIf both reverts are clean, then the typical use case I can think of is\nthat a prerequisite was missing, hence the revert; after all the\nprerequisites were in place, it was then safe to roll forward, hence\nthe reland. I would hope that the full commit messages for these\nrevert commits would provide this context since without that kind of\ncommunication from the people involved, a reader is lost.\n\n> The suggestion is to change the behavior to \"Revert^N\" instead of\n> multiple Reverts one after another.\n\nThis would be replacing one kind of jargon with another, so it's not\nclear to me that it would improve matters.\n\nWith Revert / Reland, we can forget the N altogether:\n\n 1. 'Do some great thing'\n 2. 'Revert \"Do some great thing\"'\n 3. 'Reland \"Do some great thing\"'\n 4. 'Revert \"Do some great thing\"'\n 5. (etc)\n\nFor the reader of the shortlog, it's not too important how long the\nedit war has gone. The single word makes it clear what the commit\nis going to do.\n\nIn the full commit message, I would hope that people put in some\nexplanation of the story so far to explain why they have gone\nback-and-forth all these times.\n\nThanks and hope that helps,\nJonathan\n\n[1] https://groups.google.com/d/msg/repo-discuss/edl43LcyEKE/f9Q65kOcBgAJ\n[2] https://chromium-review.googlesource.com/c/infra/gerrit-plugins/chromium-behavior/+/757684\n"},{"id":"389103","messageId":"20191230203348.GA831634@ugly","threadId":"52514","inReplyTo":"20191230195915.GE57251@google.com","subject":"Re: Updating the commit message for reverts","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2019-12-30T20:33:48Z","receivedAt":"2019-12-30T20:41:26Z","isPatch":false,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"On Mon, Dec 30, 2019 at 11:59:15AM -0800, Jonathan Nieder wrote:\n>Gal Paikin wrote:\n>> The suggestion is to change the behavior to \"Revert^N\" instead of\n>> multiple Reverts one after another.\n>\n>This would be replacing one kind of jargon with another, so it's not\n>clear to me that it would improve matters.\n>\n>With Revert / Reland, we can forget the N altogether: [...]\n>\nthe irony here is that \"reland\" is of course yet more jargon. :-D\ni'd strongly suggest to use something that actually appears in standard \ndictionaries, preferentially \"reapply\".\n\n> 1. 'Do some great thing'\n> 2. 'Revert \"Do some great thing\"'\n> 3. 'Reland \"Do some great thing\"'\n> 4. 'Revert \"Do some great thing\"'\n> 5. (etc)\n>\n>For the reader of the shortlog, it's not too important how long the\n>edit war has gone. The single word makes it clear what the commit\n>is going to do.\n>\nin principle i agree, but it irks me somewhat that the summaries become \nnon-unique, as that always somewhat impacts history browsing.\n"}]}