<div dir="ltr"><div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sat, Nov 9, 2019 at 11:50 PM Joan Lluch <<a href="mailto:joan.lluch@icloud.com">joan.lluch@icloud.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div style="word-wrap:break-word"><br><div><blockquote type="cite"><div>On 10 Nov 2019, at 07:00, Mehdi AMINI <<a href="mailto:joker.eph@gmail.com" target="_blank">joker.eph@gmail.com</a>> wrote:</div><br><div><div style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><span> </span>recipe is not correct in the absolute: the delta from master does not mean it contains exactly what you want, you seem to assume that master didn't evolve between the time "patchbranch" was created.</div><br></div></blockquote></div><br><div>Hi Mehdi,</div><div><br></div><div>I’m doing it this way to make sure that master /actually/ contains “exactly what I want” (!). Of course the remote master could have evolved slightly during such steps</div></div></blockquote><div><br></div><div>I meant: your list of instructions assume that master didn't move *locally* since you branched "patchbranch".</div><div>If you're juggling with multiple patch branches at the same time, you need to make sure they are all rebased correctly on the current local master.</div><div>Your workflow may work very well for you, but I was pointing this out because someone trying to adapt it can mess up their commit fairly easily if they just follow this recipe.</div><div>Interactive rebase is much safer from this point of view.</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div style="word-wrap:break-word"><div>, but that’s a cat-and-mouse game, that I don’t think that can be easily avoided (at least to my knowledge): We use Phabricator to get patches reviewed and this implies that new commits should be based on what we post there. If master has evolved significantly since the review, then a patch update should be sent to Phabricator anyway for further review. Also tests must be run again while master continues evolving. Ultimately, the commit that gets sent to Github must be strictly based on what we got reviewed on Phabricator. My procedure only attempts to enforce that the ‘delta’ commit that I push is predictable, and exactly the one that I previously posted on Phabricator.</div><div><br></div><div>The procedure stated on the docs   <a href="https://llvm.org/docs/Phabricator.html#committing-a-change" target="_blank">https://llvm.org/docs/Phabricator.html#committing-a-change</a> suggests using ‘arcanist’ to create a ‘diferential’ branch based on master and the Phabricator revision number. The 'arc patch’ command is used for that. To my understanding, the problem is the same: creating that branch may take a few seconds, and while doing so, master can evolve too. The docs even suggest re-running the tests before pushing, which gives even more time for master to have changed.</div></div></blockquote><div><br></div><div>No: the arcanist command does not suffer from the problem I was raising.</div><div>The issue I was referring to is that your reset command will lead to *undoing* changes from master (unrelated to your branch) when you commit in the end (all the changes that are in master but not in "patchbranch").</div><div>(just try to add `git checkout master && git pull && git checkout tmp` before your `git reset `, and then look at the resulting commit).</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div style="word-wrap:break-word"><div><br></div><div>(Said that, maybe I’m not fully getting ‘git', because I only used it in the past with small teams and no need for cross-reviews, so everything was pretty straightforward: pulling what others did, merging working branches with master, eventually resolving conflicts, and then pushing)</div><div><br></div><div>Since you suggest that my “recipe” is not correct, I would appreciate that you elaborate on the correct one, which is why I opened this subject to begin with.</div></div></blockquote><div><br></div><div>Avoid `git reset` unless you are really sure that this is what you need at a given time: I would not advise `git reset` to any beginner for a "normal" workflow.</div><div>Christopher mentioned a more robust set of steps for instance, and in general `git rebase -i` is the tool: you likely want to invest in knowing it (you mentioned you had to address conflicts, but such situation will lead to resolve conflicts regardless of the workflow, yours included).</div><div><br></div><div>Best,</div><div><br></div><div>-- </div><div>Mehdi</div><div><br></div></div></div></div>