{"thread":{"id":"51216","subject":"change inside PR not reflected in the resulting squash commit","startedAt":"2019-05-31T21:03:32Z","lastAt":"2019-06-02T20:11:58Z","messageCount":2,"participants":["ARAM MALIACHI","Philip Oakley"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"376512","messageId":"CAOkV5k19NupZ3L=2QiRuYer8-gMBFM9TKuW5RR2zzHhnVmfZ6w@mail.gmail.com","threadId":"51216","inReplyTo":null,"subject":"change inside PR not reflected in the resulting squash commit","fromName":"ARAM MALIACHI","fromEmail":"maliachi.aram@gmail.com","sentAt":"2019-05-31T21:03:19Z","receivedAt":"2019-05-31T21:03:32Z","isPatch":false,"sender":{"key":"maliachi.aram@gmail.com","avatar":null},"body":"I’m an Azure DevOps support engineer for Microsoft. Prior to composing\nthis communication to the Git team I had a conversation with the\nProduct Group from Azure DevOps stating this could be expected\nbehavior, truth is they didn’t take the time to analyze this with me\nstating our PR system does nothing extra to the regular Git tool.\nHowever I’m still suspicions about it since I consider myself advanced\nin Git workflows.\nCustomer completed a PR from release/Pl4S1R4 into master. PR's id 5238.\nSeveral commits made up this PR: 15 approximately were added to the\nsource branch release/Pl4S1R4 while the PR was active.\n2 out of 15 commits made the exact same change to the exact same file\non the exact same lines. One of these two commits was committed to\nsource branch directly and the other one is a squash merge commit that\nresulted from another PR completion to source branch coming from a 3rd\nbranch while the original PR 5238 was active.\nSpecifically commits [descending order]:\n13ade36d - squash commit of a third branch coming into source branch\n42940662 - commit made directly to the source branch\nSpecifically, both commits include along many changes the following\nchange to file /src/components/Layout/Menu.vue on line 80 and line 91\nOriginal version of file line 80 and 91:\n80   {{menuOptions.NETWORK}} {{'v'}}\n91   {{network.name}} {{'v'}}\nResulting version of the file in line 80 and 91 for both commits:\n80   {{menuOptions.NETWORK}}\n91   {{network.name}}\nThe weird thing which raises a flag for me is that making the same\nchange to the exact same file 2 times wouldn't even be possible at all\nsince first change would make its current status the desired one and\ntherefore the next commit with the same change wouldn't even highlight\nthe lines as change. I believe this could be due to the nature of the\nsecond commit coming from a PR completion [squash] where maybe the\nfile was kept unchanged.\nWhat I would expect here is that the PR would read the file and would\nignore the second change made by squash commit since this exact change\nhad already taken place previously.\nAbove all this despite the PR having 2 commits highlighting the exact\nsame change on a file, the resulting squash commit ignored them both\nand showed the lines 80 and 91 in the affected file as untouched.\nI carefully verified with cx that no other commit would 'revert' this\ntwo changes or even edit the file /src/components/Layout/Menu.vue\nagain so it seems like the system indeed ignored them both on its own.\nNote that other changes to different lines of the affected file were\nrecorded correctly in the resulting squash commit once PR was\ncompleted.\n\nDue to the complexity of cx's environment, I haven't been able to\nreproduce the issue myself, but the proof is inside the PR stored in\ncx's Project.\n\n--\n"},{"id":"376555","messageId":"7a6d8cf4-91e8-7cf2-bcc6-9b3a1a2f4382@iee.org","threadId":"51216","inReplyTo":"CAOkV5k19NupZ3L=2QiRuYer8-gMBFM9TKuW5RR2zzHhnVmfZ6w@mail.gmail.com","subject":"Re: change inside PR not reflected in the resulting squash commit","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2019-06-02T20:11:55Z","receivedAt":"2019-06-02T20:11:58Z","isPatch":false,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"Hi Maliachi,\nIt's a quiet night. I'll have a go...\nI have spaced out your text..\n\nOn 31/05/2019 22:03, ARAM MALIACHI wrote:\n> I’m an Azure DevOps support engineer for Microsoft. Prior to composing\n> this communication to the Git team I had a conversation with the\n> Product Group from Azure DevOps stating this could be expected\n> behavior, truth is they didn’t take the time to analyze this with me\n> stating our PR system does nothing extra to the regular Git tool.\nPR systems can add extra processing steps which allow the flow to be \nmore complex that at first glance. Not sure which PR admin system this \none is..\n> However I’m still suspicions about it since I consider myself advanced\n> in Git workflows.\n> Customer completed a PR from release/Pl4S1R4 into master. PR's id 5238.\nThis will be a series at a specific base commit.\n> Several commits made up this PR: 15 approximately were added to the\n> source branch release/Pl4S1R4 while the PR was active.\nextras, extras, opportunity for confusion...\n> 2 out of 15 commits made the exact same change to the exact same file\n> on the exact same lines.\nBe wary of terminology here. If the _final_ series was linear without \nmerges, then such diffs don't exist, hence the assumption is fallacious.\n>   One of these two commits was committed to\n> source branch directly\nSo you have a 'side' branch change, and the PR later merges with this.\n>   and the other one is a squash merge commit that\n> resulted from another PR completion\nA further 'side' branch (for the squash series), and later merges into \nthe PR.\n>   to source branch coming from a 3rd\n> branch while the original PR 5238 was active.\n\n> Specifically commits [descending order]:\n> 13ade36d - squash commit of a third branch coming into source branch\n> 42940662 - commit made directly to the source branch\n> Specifically, both commits include along many changes the following\n> change to file /src/components/Layout/Menu.vue on line 80 and line 91\n> Original version of file line 80 and 91:\n> 80   {{menuOptions.NETWORK}} {{'v'}}\n> 91   {{network.name}} {{'v'}}\n> Resulting version of the file in line 80 and 91 for both commits:\n> 80   {{menuOptions.NETWORK}}\n> 91   {{network.name}}\nI.e. you have clean (non) merges! they all agree on the right final \nanswer. These merges have other content, so have sufficient diffs to \nlook interestingly complete, while their earlier commits on the side \nhave diffs that give you to think that there are identical changes \nwithin the PR (rather than being outside of the series)\n\nHave a look at the new \"--rebase-merges\" (and the discussion threads) \nfor some of the issues about bringing in disparate changes via merging in.\n> The weird thing which raises a flag for me is that making the same\n> change to the exact same file 2 times wouldn't even be possible at all\n> since first change would make its current status the desired one and\n> therefore the next commit with the same change wouldn't even highlight\n> the lines as change. I believe this could be due to the nature of the\n> second commit coming from a PR completion [squash] where maybe the\n> file was kept unchanged.\nI guess it is a time flow disconnect (things you thought static \nchanged), and perhaps history simplification (the graphs missed \nimportant bits) that has caused the confusion (assuming I have \nunderstood the problem statement, and guessed right as to the \nawkwardness of explaining it)\n> What I would expect here is that the PR would read the file and would\n> ignore the second change made by squash commit since this exact change\n> had already taken place previously.\n> Above all this despite the PR having 2 commits highlighting the exact\n> same change on a file, the resulting squash commit ignored them both\n> and showed the lines 80 and 91 in the affected file as untouched.\n> I carefully verified with cx that no other commit would 'revert' this\n> two changes or even edit the file /src/components/Layout/Menu.vue\n> again so it seems like the system indeed ignored them both on its own.\n> Note that other changes to different lines of the affected file were\n> recorded correctly in the resulting squash commit once PR was\n> completed.\n>\n> Due to the complexity of cx's environment, I haven't been able to\n> reproduce the issue myself, but the proof is inside the PR stored in\n> cx's Project.\nIt should be able to make a toy example with a similar profile. just \nmake sure that you have a full DAG covering the three PR series (master \nbranch initial state, PR5238, squash-PR, direct commit PR, final state) \nYou'll only need a few commits on each. It will be a worthwhile learning \nexercise for you, and to show to colleagues.\n--\nPhilip\nPS I could be wrong.\n"}]}