{"thread":{"id":"43986","subject":"Fixup of a fixup not working right","startedAt":"2016-09-02T19:24:35Z","lastAt":"2016-09-07T11:31:53Z","messageCount":10,"participants":["Robert Dailey","Philip Oakley","Junio C Hamano","Johannes Schindelin"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"300905","messageId":"CAHd499AQFDRps6POF2xuUjbYv5DJYxt3DA8aFFArXF=qQEz_CA@mail.gmail.com","threadId":"43986","inReplyTo":null,"subject":"Fixup of a fixup not working right","fromName":"Robert Dailey","fromEmail":"rcdailey.lists@gmail.com","sentAt":"2016-09-02T19:24:28Z","receivedAt":"2016-09-02T19:24:35Z","isPatch":false,"sender":{"key":"rcdailey.lists@gmail.com","avatar":null},"body":"Suppose I have a branch with 4 commits, in the following order (as you\nmight see during interactive rebase):\n\npick 123 Original Change\npick 789 fixup! Original Change\npick 456 Some Other Thing\npick abc fixup! fixup! Original Change\n\nHowever, let's say the first commit is already pushed upstream on a\ntopic branch. Since there are multiple developers on this topic\nbranch, I do not want to rebase right now. Instead, I want to document\nfuture fixups via fixup commits and then when we're ready to merge, do\na final rebase prior to the merge to master to clean things up after\nwe're all done collaborating.\n\nFor this specific situation, since the first commit is already pushed,\nI want to perform a fixup on the 1st fixup commit. When I perform an\ninteractive rebase against upstream topic, I get the following:\n\npick 789 fixup! Original Change\npick 456 Some Other Thing\npick abc fixup! fixup! Original Change\n\nThe tip commit (abc in this case) is not marked as a fixup. What I\nexpect to see is:\n\npick 789 fixup! Original Change\nfixup abc fixup! fixup! Original Change\npick 456 Some Other Thing\n\nIs this by design, or a defect? I assumed that Git would only look at\nthe first occurrence of \"fixup!\" and treat everything else after as\nthe commit description to match. But it seems in this case that it\nstops at the last occurrence of \"fixup!\", which would explain why it\nisn't matching in the interactive rebase. I haven't looked at the\ncode, though.\n\nThoughts? Also I'm perfectly willing to accept feedback involving me\njust using the feature wrong or as not intended. Thanks in advance.\n"},{"id":"300920","messageId":"55512A8927384A0790DDC7F526B09053@PhilipOakley","threadId":"43986","inReplyTo":"CAHd499AQFDRps6POF2xuUjbYv5DJYxt3DA8aFFArXF=qQEz_CA@mail.gmail.com","subject":"Re: Fixup of a fixup not working right","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2016-09-02T22:24:16Z","receivedAt":"2016-09-02T22:24:22Z","isPatch":false,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Robert Dailey\" <rcdailey.lists@gmail.com>\n> Suppose I have a branch with 4 commits, in the following order (as you\n> might see during interactive rebase):\n>\n> pick 123 Original Change\n> pick 789 fixup! Original Change\n> pick 456 Some Other Thing\n> pick abc fixup! fixup! Original Change\n>\n> However, let's say the first commit is already pushed upstream on a\n> topic branch. Since there are multiple developers on this topic\n> branch, I do not want to rebase right now. Instead, I want to document\n> future fixups via fixup commits and then when we're ready to merge, do\n> a final rebase prior to the merge to master to clean things up after\n> we're all done collaborating.\n>\n> For this specific situation, since the first commit is already pushed,\n> I want to perform a fixup on the 1st fixup commit. When I perform an\n> interactive rebase against upstream topic, I get the following:\n>\n> pick 789 fixup! Original Change\n> pick 456 Some Other Thing\n> pick abc fixup! fixup! Original Change\n>\n> The tip commit (abc in this case) is not marked as a fixup. What I\n> expect to see is:\n>\n> pick 789 fixup! Original Change\n> fixup abc fixup! fixup! Original Change\n> pick 456 Some Other Thing\n>\n> Is this by design, or a defect? I assumed that Git would only look at\n> the first occurrence of \"fixup!\" and treat everything else after as\n> the commit description to match. But it seems in this case that it\n> stops at the last occurrence of \"fixup!\", which would explain why it\n> isn't matching in the interactive rebase. I haven't looked at the\n> code, though.\n\nAs I understand this it's implied by design. The issue is that the rebase is \nlooking for that named commit within its current rebase range, and can't \nfind it, so ignores it.\n\nThere is a separate issue that all the fixup! fixup! messages are \nessentially treated as being concatenations of the original fixup!, no \nmatter how many time the fiup is present.\n\nIn the mean time you should reword those commit messages as being \n'bug-fixes' as they are (you say) already upstream and hence published. You \ncan make the first as a bug-fix and the following ones a fixup!s.\n\n>\n> Thoughts? Also I'm perfectly willing to accept feedback involving me\n> just using the feature wrong or as not intended. Thanks in advance.\n> \n\n"},{"id":"300936","messageId":"xmqq60qdsoxj.fsf@gitster.mtv.corp.google.com","threadId":"43986","inReplyTo":"55512A8927384A0790DDC7F526B09053@PhilipOakley","subject":"Re: Fixup of a fixup not working right","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-03T02:22:00Z","receivedAt":"2016-09-03T02:22:08Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Philip Oakley\" <philipoakley@iee.org> writes:\n\n> As I understand this it's implied by design. The issue is that the\n> rebase is looking for that named commit within its current rebase\n> range, and can't find it, so ignores it.\n>\n> There is a separate issue that all the fixup! fixup! messages are\n> essentially treated as being concatenations of the original fixup!, no\n> matter how many time the fiup is present.\n\nThey can be handled separately, but they come from the same \"design\"\nthat could be improved.  When the \"original\" is not in the range to\nbe rebased for whatever reason (including the most likely one, i.e.\nit has already graduated to become part of the public history), the\nbest thing the user could do at that point may be, as you suggested\nto Robert in your message, to turn the \"fixup! original\" that did\nnot make in time before \"original\" hit the public record into a\nstandalone \"fix original\" follow-up change, and then to squash\nsubsequent \"fixup! fixup! original\" (and other \"fixup! original\",\ntoo) into that commit.  And a good direction forward may be to see\nif \"rebase -i\" can be taught to be more helpful for the user who\nwants to do that.\n\nPerhaps a change like this to \"rebase -i\":\n\n - The search for \"original\" when handling \"pick fixup! original\",\n   when it does not find \"original\", could turn it into \"reword\n   fixup! original\" without changing its position in the instruction\n   sequence.\n\n - The search for \"original\" when handling \"pick fixup! fixup!\n   original\", could be (probably unconditionally) changed to look\n   for \"fixup! original\" to amend, instead of looking for \"original\"\n   as the current code (this is your \"separate issue\").  The same\n   \"if the commit to be amended is not found, turn it into reword\"\n   rule from the above applies to this one, too.\n\nmay be an improvement?\n"},{"id":"300948","messageId":"CAHd499Bd7CHZ-O8HLd_iNnVNxz1C1=vO-4kxD80sEgBiesrMXg@mail.gmail.com","threadId":"43986","inReplyTo":"xmqq60qdsoxj.fsf@gitster.mtv.corp.google.com","subject":"Re: Fixup of a fixup not working right","fromName":"Robert Dailey","fromEmail":"rcdailey.lists@gmail.com","sentAt":"2016-09-03T14:12:43Z","receivedAt":"2016-09-03T14:13:28Z","isPatch":false,"sender":{"key":"rcdailey.lists@gmail.com","avatar":null},"body":"On Fri, Sep 2, 2016 at 9:22 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Perhaps a change like this to \"rebase -i\":\n>\n>  - The search for \"original\" when handling \"pick fixup! original\",\n>    when it does not find \"original\", could turn it into \"reword\n>    fixup! original\" without changing its position in the instruction\n>    sequence.\n\nIf it did this, then later when I'm ready to merge and all\ncontributions upstream are completed, I would most likely lose track\nof which original commit to fixup to. This would only be an issue if\nyou gave the option to reword the \"fixup! original\". Rewording it, to\nme, inherently means you no longer want to meld it into another commit\nand instead intend on it being independent (looking at this\nsemantically, without relying on translating the commit message\nitself, even though it may clearly tell me to squash it to something\nelse, that's not as reliable as the fixup! mechanics IMHO).\n\n>  - The search for \"original\" when handling \"pick fixup! fixup!\n>    original\", could be (probably unconditionally) changed to look\n>    for \"fixup! original\" to amend, instead of looking for \"original\"\n>    as the current code (this is your \"separate issue\").  The same\n>    \"if the commit to be amended is not found, turn it into reword\"\n>    rule from the above applies to this one, too.\n\nSo this is mostly for my education, since I don't see a difference\nfrom a user-standpoint. Why would \"fixup! fixup! original\" look for\n\"original\" instead of \"fixup! original\"? As far as I can see, the\nbehavior would be the same and the order would be retained since you\nare essentially \"chaining\" the fixups together this way. This also\ngives you the behavior you want directly because the algorithm will\nnaturally tie to \"fixup! original\" even if \"original\" doesn't exist,\nbecause Git would expect that \"fixup! original\" will automatically\nmanage its own order, and that subsequent processed nested \"fixup!\"\ncommits would not need to depend on any other commits.\n"},{"id":"300959","messageId":"539B8BADEBFE4539ADC8B7EB60B23B5B@PhilipOakley","threadId":"43986","inReplyTo":"CAHd499Bd7CHZ-O8HLd_iNnVNxz1C1=vO-4kxD80sEgBiesrMXg@mail.gmail.com","subject":"Re: Fixup of a fixup not working right","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2016-09-03T22:33:59Z","receivedAt":"2016-09-03T22:34:06Z","isPatch":false,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"Hi Robert,\nFrom: \"Robert Dailey\" <rcdailey.lists@gmail.com>\n> On Fri, Sep 2, 2016 at 9:22 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Perhaps a change like this to \"rebase -i\":\n>>\n>>  - The search for \"original\" when handling \"pick fixup! original\",\n>>    when it does not find \"original\", could turn it into \"reword\n>>    fixup! original\" without changing its position in the instruction\n>>    sequence.\n>\n[..]\n> So this is mostly for my education, since I don't see a difference\n> from a user-standpoint.\n\nThis was a problem in the past.\n\n>   Why would \"fixup! fixup! original\" look for\n> \"original\" instead of \"fixup! original\"? As far as I can see, the\n> behavior would be the same and the order would be retained since you\n> are essentially \"chaining\" the fixups together this way.\n\nThe patch that introduced the effect is 22c5b13 (rebase -i: handle fixup! \nfixup! in --autosquash, 2013-06-27). At the time it was possible that the \ncommits would be re-ordered based on the full multi-fixup message, and so, \nif you thought you'd corrected a poor fixup, rather correcting the original, \nthe conceptual order would change. While the combined diffs still notionally \nadd to the same effect, the changes to the context lines may mean they don't \napply cleanly - keeping them in order makes for a clean application. The \nsolution was to ignore multiple fixup!s at the start.\n\n\n> This also\n> gives you the behavior you want directly because the algorithm will\n> naturally tie to \"fixup! original\" even if \"original\" doesn't exist,\n> because Git would expect that \"fixup! original\" will automatically\n> manage its own order, and that subsequent processed nested \"fixup!\"\n> commits would not need to depend on any other commits.\n\nThere is a question as to whether the commit you pushed upstream, is classed \nas 'published' and immutable, or still part of the review and modification \nprocess. At the moment the presumption, in general, is that it would be the \nformer, so that you can't fixup the original. I don't see that changing, \nhowever Junio's suggestion that these extra fixups are 'reworded' so that \nthey will rebase (--autosquash) locally to become one commit titled \nsomething like \"fix! original\", and then a final rebase that rewords that \ncommit back to \"fixup! \" for pushing upstream to the 'review' process, where \nthey can request that it be 'fixed';-) \n\n"},{"id":"300971","messageId":"alpine.DEB.2.20.1609040923390.129229@virtualbox","threadId":"43986","inReplyTo":"xmqq60qdsoxj.fsf@gitster.mtv.corp.google.com","subject":"Re: Fixup of a fixup not working right","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-09-04T07:36:46Z","receivedAt":"2016-09-04T07:37:11Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio & Philip,\n\nOn Fri, 2 Sep 2016, Junio C Hamano wrote:\n\n> \"Philip Oakley\" <philipoakley@iee.org> writes:\n> \n> > As I understand this it's implied by design. The issue is that the\n> > rebase is looking for that named commit within its current rebase\n> > range, and can't find it, so ignores it.\n> >\n> > There is a separate issue that all the fixup! fixup! messages are\n> > essentially treated as being concatenations of the original fixup!, no\n> > matter how many time the fiup is present.\n> \n> They can be handled separately, but they come from the same \"design\"\n> that could be improved.  When the \"original\" is not in the range to\n> be rebased for whatever reason (including the most likely one, i.e.\n> it has already graduated to become part of the public history), the\n> best thing the user could do at that point may be, as you suggested\n> to Robert in your message, to turn the \"fixup! original\" that did\n> not make in time before \"original\" hit the public record into a\n> standalone \"fix original\" follow-up change, and then to squash\n> subsequent \"fixup! fixup! original\" (and other \"fixup! original\",\n> too) into that commit.  And a good direction forward may be to see\n> if \"rebase -i\" can be taught to be more helpful for the user who\n> wants to do that.\n> \n> Perhaps a change like this to \"rebase -i\":\n> \n>  - The search for \"original\" when handling \"pick fixup! original\",\n>    when it does not find \"original\", could turn it into \"reword\n>    fixup! original\" without changing its position in the instruction\n>    sequence.\n> \n>  - The search for \"original\" when handling \"pick fixup! fixup!\n>    original\", could be (probably unconditionally) changed to look\n>    for \"fixup! original\" to amend, instead of looking for \"original\"\n>    as the current code (this is your \"separate issue\").  The same\n>    \"if the commit to be amended is not found, turn it into reword\"\n>    rule from the above applies to this one, too.\n> \n> may be an improvement?\n\nI would be *very* careful with such a change.\n\nThe point is that fixup! messages are really special, and are always\nintended to be squashed into the referenced commit *before* the latter\nhits `master`.\n\nThe entire design of the fixup! feature (using the commit subject as\nidentifier, which is only \"unique enough\" in a topic branch that is still\nbeing developed) points to that.\n\nI am fairly certain that we would run into tons of problems if we diluted\nthe concept of fixup! commits by changing the design so that fixup!\ncommits all of a sudden become their own, \"real\" commits that can be fixed\nup themselves, as much of the current code simply does not expect that.\n\nIn short, I am opposed to this change.\n\nAnd even if I am overruled, I would strongly suggest to implement this on\ntop of my rebase-i-extra branch (i.e. in the rebase--helper instead of the\nshell script) to avoid double churn.\n\nCiao,\nJohannes\n"},{"id":"300988","messageId":"8F7471262D1C45D9A7BD1C29277004AD@PhilipOakley","threadId":"43986","inReplyTo":"alpine.DEB.2.20.1609040923390.129229@virtualbox","subject":"Re: Fixup of a fixup not working right","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2016-09-04T11:47:55Z","receivedAt":"2016-09-04T11:48:01Z","isPatch":false,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Johannes Schindelin\" <Johannes.Schindelin@gmx.de>\n> Hi Junio & Philip,\n>\n> On Fri, 2 Sep 2016, Junio C Hamano wrote:\n>\n>> \"Philip Oakley\" <philipoakley@iee.org> writes:\n>>\n>> > As I understand this it's implied by design. The issue is that the\n>> > rebase is looking for that named commit within its current rebase\n>> > range, and can't find it, so ignores it.\n>> >\n>> > There is a separate issue that all the fixup! fixup! messages are\n>> > essentially treated as being concatenations of the original fixup!, no\n>> > matter how many time the fiup is present.\n>>\n>> They can be handled separately, but they come from the same \"design\"\n>> that could be improved.  When the \"original\" is not in the range to\n>> be rebased for whatever reason (including the most likely one, i.e.\n>> it has already graduated to become part of the public history), the\n>> best thing the user could do at that point may be, as you suggested\n>> to Robert in your message, to turn the \"fixup! original\" that did\n>> not make in time before \"original\" hit the public record into a\n>> standalone \"fix original\" follow-up change, and then to squash\n>> subsequent \"fixup! fixup! original\" (and other \"fixup! original\",\n>> too) into that commit.  And a good direction forward may be to see\n>> if \"rebase -i\" can be taught to be more helpful for the user who\n>> wants to do that.\n>>\n>> Perhaps a change like this to \"rebase -i\":\n>>\n>>  - The search for \"original\" when handling \"pick fixup! original\",\n>>    when it does not find \"original\", could turn it into \"reword\n>>    fixup! original\" without changing its position in the instruction\n>>    sequence.\n>>\n>>  - The search for \"original\" when handling \"pick fixup! fixup!\n>>    original\", could be (probably unconditionally) changed to look\n>>    for \"fixup! original\" to amend, instead of looking for \"original\"\n>>    as the current code (this is your \"separate issue\").  The same\n>>    \"if the commit to be amended is not found, turn it into reword\"\n>>    rule from the above applies to this one, too.\n>>\n>> may be an improvement?\n>\n> I would be *very* careful with such a change.\n\nI agree about the need for care. The use case must be well understood.\n\n> The point is that fixup! messages are really special, and are always\n> intended to be squashed into the referenced commit *before* the latter\n> hits `master`.\n\nI think it's here that we have the hidden use case. I agree that all fixups \nshould be squashed before they hit the blessed golden  repository.\n\nI suspect that some use cases have intermediate repositories that contain a \n'master' branch (it's just a name ;-) that isn't blessed and golden, e.g. at \nthe team review repo level. In such cases it is possible for a fixup! to be \npassed up as part of the review, though it's not the current \nnorm/expectation.\n\n>\n> The entire design of the fixup! feature (using the commit subject as\n> identifier, which is only \"unique enough\" in a topic branch that is still\n> being developed) points to that.\n>\n> I am fairly certain that we would run into tons of problems if we diluted\n> the concept of fixup! commits by changing the design so that fixup!\n> commits all of a sudden become their own, \"real\" commits that can be fixed\n> up themselves, as much of the current code simply does not expect that.\n\nWe already had that. the commit 22c5b13 (rebase -i: handle fixup! fixup! \nin --autosquash, 2013-06-27) was an attempt to work around misunderstandings \nabout what fixed what.\n\nIn Robert's scenario (IIUC) that patch was too aggressive for the case where \nthe original commit is not part of the rebase. The patch lept in a little \ntoo early in the processing so as to pretend that if it saw repeated \"fixup! \n\" strings at the start of the messages it pretented there was only one, so \nthat they all applied in the original sequence.\n\nI _think_ that the right approach would be to just bring such fixups \ntogether in the to-do list (\"as normal\"), but still have the minimal common \ncommit message string still present, so that it would, in this case, still \nhave a resulting squashed commit that starts \"!fixup \". It's important to be \nmoderately lenient to user choices - they may know something we don't, or at \nleast accept that their use case is 'unusual' [1]\n\n>\n> In short, I am opposed to this change.\n\nIt's not like G4W doesn't need fixup!s on the side branches e.g. 5eaffe9 \n(\"fixup! Handle new t1501 test case properly with MinGW\", 2016-07-12)\n\n> And even if I am overruled, I would strongly suggest to implement this on\n> top of my rebase-i-extra branch (i.e. in the rebase--helper instead of the\n> shell script) to avoid double churn.\n\nI definitely agree there.\n\n>\n> Ciao,\n> Johannes\n>\n--\nPhilip\n[1] The removal of the \"theirs\" merge strategy is one I'd add to that list. \nCalling it, for example, \"reversed\" would have kept it available while \nreduced it's visibility. See http://marc.info/?l=git&m=121637513604413&w=2 \n\n"},{"id":"301082","messageId":"alpine.DEB.2.20.1609050950130.129229@virtualbox","threadId":"43986","inReplyTo":"8F7471262D1C45D9A7BD1C29277004AD@PhilipOakley","subject":"Re: Fixup of a fixup not working right","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-09-05T07:53:30Z","receivedAt":"2016-09-05T07:54:00Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Philip,\n\nOn Sun, 4 Sep 2016, Philip Oakley wrote:\n\n> From: \"Johannes Schindelin\" <Johannes.Schindelin@gmx.de>\n> \n> > The point is that fixup! messages are really special, and are always\n> > intended to be squashed into the referenced commit *before* the latter\n> > hits `master`.\n> \n> I think it's here that we have the hidden use case. I agree that all fixups\n> should be squashed before they hit the blessed golden  repository.\n> \n> I suspect that some use cases have intermediate repositories that\n> contain a 'master' branch (it's just a name ;-) that isn't blessed and\n> golden, e.g. at the team review repo level. In such cases it is possible\n> for a fixup! to be passed up as part of the review, though it's not the\n> current norm/expectation.\n\nIn such a case (which can totally arise when criss-crossing Pull Requests\non GitHub, for example, where a Pull Request's purpose may be to fix up\ncommits in another Pull Request before the latter is merged), the most\nappropriate course of action is... to not reorder the fixup!s prematurely.\n\n> > In short, I am opposed to this change.\n> \n> It's not like G4W doesn't need fixup!s on the side branches e.g. 5eaffe9\n> (\"fixup! Handle new t1501 test case properly with MinGW\", 2016-07-12)\n\nYeah, well, Git for Windows' `master` branch is special, in that it is\nconstantly rebased (as \"merging rebases\", to keep fast-forwardability). I\nwould not necessarily use Git for Windows as a role model in this respect.\n\nCiao,\nDscho\n"},{"id":"301228","messageId":"6C5AB39424064692A416A490B861D330@PhilipOakley","threadId":"43986","inReplyTo":"alpine.DEB.2.20.1609050950130.129229@virtualbox","subject":"Re: Fixup of a fixup not working right","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2016-09-06T19:02:16Z","receivedAt":"2016-09-06T19:02:31Z","isPatch":false,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Johannes Schindelin\" <Johannes.Schindelin@gmx.de>\n> Hi Philip,\n>\n> On Sun, 4 Sep 2016, Philip Oakley wrote:\n>\n>> From: \"Johannes Schindelin\" <Johannes.Schindelin@gmx.de>\n>>\n>> > The point is that fixup! messages are really special, and are always\n>> > intended to be squashed into the referenced commit *before* the latter\n>> > hits `master`.\n>>\n>> I think it's here that we have the hidden use case. I agree that all \n>> fixups\n>> should be squashed before they hit the blessed golden  repository.\n>>\n>> I suspect that some use cases have intermediate repositories that\n>> contain a 'master' branch (it's just a name ;-) that isn't blessed and\n>> golden, e.g. at the team review repo level. In such cases it is possible\n>> for a fixup! to be passed up as part of the review, though it's not the\n>> current norm/expectation.\n>\n> In such a case (which can totally arise when criss-crossing Pull Requests\n> on GitHub, for example, where a Pull Request's purpose may be to fix up\n> commits in another Pull Request before the latter is merged), the most\n> appropriate course of action is... to not reorder the fixup!s prematurely.\n\nWe just need to be careful about that plural just there.\n\nIf it is multiple fixup!s for the same commit, then I believe they should be \ngrouped together at the same point as the first fixup! commit (in their \noriginal order).\n\nIf they are for different commits, then they should stay in their place in \nthe commit series (for their first occurrence, then rule 1 applies)\n\n>\n>> > In short, I am opposed to this change.\n>>\n>> It's not like G4W doesn't need fixup!s on the side branches e.g. 5eaffe9\n>> (\"fixup! Handle new t1501 test case properly with MinGW\", 2016-07-12)\n\nI note that you don't have two fixup!s for that commit\n\n> Yeah, well, Git for Windows' `master` branch is special, in that it is\n> constantly rebased (as \"merging rebases\", to keep fast-forwardability). I\n> would not necessarily use Git for Windows as a role model in this respect.\n\nI don't see GfW as 'special', rather as being a representative of a broader \nrealpolitik where some of the rugged individualism of open source is \nmoderated in some way or another.\n\n> Ciao,\n> Dscho\n>\n--\nPhilip \n\n"},{"id":"301256","messageId":"alpine.DEB.2.20.1609071327290.129229@virtualbox","threadId":"43986","inReplyTo":"6C5AB39424064692A416A490B861D330@PhilipOakley","subject":"Re: Fixup of a fixup not working right","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-09-07T11:31:35Z","receivedAt":"2016-09-07T11:31:53Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Philip,\n\nOn Tue, 6 Sep 2016, Philip Oakley wrote:\n\n> From: \"Johannes Schindelin\" <Johannes.Schindelin@gmx.de>\n> >\n> > On Sun, 4 Sep 2016, Philip Oakley wrote:\n> >\n> > > I suspect that some use cases have intermediate repositories that\n> > > contain a 'master' branch (it's just a name ;-) that isn't blessed\n> > > and golden, e.g. at the team review repo level. In such cases it is\n> > > possible for a fixup! to be passed up as part of the review, though\n> > > it's not the current norm/expectation.\n> >\n> > In such a case (which can totally arise when criss-crossing Pull\n> > Requests on GitHub, for example, where a Pull Request's purpose may be\n> > to fix up commits in another Pull Request before the latter is\n> > merged), the most appropriate course of action is... to not reorder\n> > the fixup!s prematurely.\n> \n> We just need to be careful about that plural just there.\n> \n> If it is multiple fixup!s for the same commit, then I believe they should be\n> grouped together at the same point as the first fixup! commit (in their\n> original order).\n\nWe should they be grouped together? In the final rebase (the one that\nactually also includes the commit that is to be rewritten), they will be\ngrouped together anyway.\n\nAnd it is not like users cannot regroup manually if they choose to perform\nan intermediate rebase. In that case, the user will also choose whether\nshe wants to simply regroup the fixups, or squash them into a single\nfixup, too.\n\n> > > > In short, I am opposed to this change.\n> > >\n> > > It's not like G4W doesn't need fixup!s on the side branches e.g.\n> > > 5eaffe9 (\"fixup! Handle new t1501 test case properly with MinGW\",\n> > > 2016-07-12)\n> \n> I note that you don't have two fixup!s for that commit\n\nNot for that one, no. But there have been cases where I had to add two or\nmore fixups for the same commit, in preparation for the next merging\nrebase.\n\n> > Yeah, well, Git for Windows' `master` branch is special, in that it is\n> > constantly rebased (as \"merging rebases\", to keep\n> > fast-forwardability). I would not necessarily use Git for Windows as a\n> > role model in this respect.\n> \n> I don't see GfW as 'special', rather as being a representative of a\n> broader realpolitik where some of the rugged individualism of open\n> source is moderated in some way or another.\n\nSure, it is an example of a project that needs to solve the problem where\npatch series are accumulated, to be submitted to an upstream project, and\nso we have to keep fast-forwardability at the same time as we have to\nrebase.\n\nBut Git for Windows is special in the way it solves the problem. I am not\naware of anybody else performing merging rebases.\n\nCiao,\nDscho\n"}]}