Two ways to get an accepted patch into the master branch?

(Niels Dekker) #1

Just noticed there’s a slight difference between the way @matt.mccormick and @hjmjohnson moved a commit from a pull request of mine into the master: https://github.com/InsightSoftwareConsortium/ITK/commits/master

With Matt’s approach, there appears a separate “Merge pull request” commit (“thewtex committed”), as well as my proposed commit (“N-Dekker committed”). With Hans, there appears only one commit, saying “N-Dekker authored and hjmjohnson committed”.

It seems to me that both approaches have their pro’s and cons. I like Matt’s approach slightly better, as it clearly indicates the merge date. (Which might be quite different from the original date of the proposed commit). But others might say that Matt’s approach pollutes the log history :open_mouth:

No big deal, of course, just something I noticed. :slight_smile:

(Matt McCormick) #2

A keen observation, @Niels_Dekker :eagle:

To explain what is happening:

When a branch is created from the tip of master and merged in before other commits have been added to master, then no other commits are necessary. However, if other commits have been merged to master in the meantime, then Git creates an additional merge commit. Sometimes this commit will also contain merge conflict resolution content that was automated or manually resolved.

(Niels Dekker) #3

Thanks for your explanation, Matt. But if it’s not a disagreement between your and Hans’ approach, I guess my observation wasn’t that keen :upside_down_face:

Do I understand correctly that if I would actually want to have just a single commit for my pull request, I should fetch upstream and rebase my branch, just before the review-push?

(Matt McCormick) #4

Right. :+1: