3

I have two feature branches that were recently merged, and I'm trying to examine the diffs as an informal code review after-the-fact.

feature1 with about 50 commits was merged into master and a merge commit was made on master. I had reviewed those commits and was satisfied with them. The merge was straight forward easy as master was not ahead.

Then master/feature1 (now ahead) was merged into feature2.

               master,feature1
...------------------o
                    / \
...-------o-o-o-----   \
                        \ feature2
...o-o-o-----------------o 

When I look at the diff on the merge commit on feature2, I see all the changes from 50 commits of feature1 being applied to the feature2 branch. A small number of those changes would have been in conflict requiring manual intervention. How can I review those conflict resolution decisions without having to re-review the change set of the last 50 commits on feature1?

Or have I made an incorrect assumption that the non-conflicting changes are safe, and I should review the whole lot?

(I'm quite nervous about this merge. Somebody else has already added another commit on feature2 saying that it adds some lines that were missed from the merge. How do things get missed? How do I find other things that might have been missed?)

  • You can produce the same merge in different directory/commit without resolving conflicts and get diff between merged version and this one – RiaD Oct 3 '19 at 8:17
1

Have you considered another approach? Make a new branch, feature 3, from the master with feature 1 already merged to master. Re-implement your feature 2 in that branch.

Sometimes this may be faster then reviewing all the changes.

You already did the "think work" for feature 2 and you can reuse a lot of code from the feature 2 branch.

  • Thanks. Not practical in this instance, but could be a good idea for another time. – Jeremy Hunt Oct 3 '19 at 11:35
0

Thanks to @RiaD for the comment:

You can produce the same merge in different directory/commit without resolving conflicts and get diff between merged version and this one

This worked for me - I made a new branch from the same merge point and merged in master. I left the conflict markers in the file and ran git diff master. I could see where each conflict occured in green. The manual changes that the merger had done were in red.

Lastly, I completed the merge the way I thought it should have been done and took another diff.

0

You should never have conflicts when merging to master. The way to avoid this is to first merge master into the feature branch.

Resolve the conflicts in that feature branch. Which are (should be) limited to where that particular feature overlaps with others.

Then your merge to master will then be conflict free.

You should not assume that auto resolved conflicts are safe, but doing these merges on your feature branch allows you to test the result before merging to master.

A completely non conflicting merge is 'safe' as it is just updating one branch to the state of another.

  • I don't think this answers the question. The question wasn't about merging to master. – Pieter B Oct 3 '19 at 11:17
  • you are right. which is werid because the resolved commits shouldnt produce the same conflicts – Ewan Oct 3 '19 at 11:21
  • I voted your answer up because it was still helpful - when you talked about not assuming that the changes merged from master into feature2 that weren't flagged as conflicts were safe. I thought about it some more, and of course they aren't. For instance, feature 1, which is now on master, might have completely replaced and DELETED a class that the in-progress feature2 was relying on heavily. Of course all the changes need to be reviewed. – Jeremy Hunt Oct 3 '19 at 11:32
  • we should prob not say "safe" at all, just "not introducing more change" – Ewan Oct 3 '19 at 11:35
  • but my real confusion now is, do you see the same conflicts twice? ie feature 2 has to resolve conflicts between f1 and master (you shouldnt) or are you coding two sets of conflicting changes. ie feature 2 conflicts with the changes in feature 1 (no way to get around this) – Ewan Oct 3 '19 at 11:37

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.