Git history, branch merges and rebasing

Justin Obara obara.justin at gmail.com
Tue Mar 8 18:18:36 UTC 2011


Hi Michelle,

Nice experimentation. It's pretty detailed. Here are some comments and questions. 

In regards to losing code review comments, what would happen if Golam were to delete his branch/fork, would we still see the code review comments? 
By using rebase, the commits are pulled out of chronological order. You can see this in your examples where Golam's commit on Feb 23 is on top of my commit on Mar 6. This might make it confusing when looking through the history. 
Another option would be to just go back to the patch/review process. 
Good information about creating and applying patches
http://ariejan.net/2009/10/26/how-to-create-and-apply-a-patch-with-git/
githubs help shows how to use git am with pull requests
http://help.github.com/pull-requests/

I think my preference would still be to use merge. You can do "git log --no-merges" to view the logs and not see the merge commits. This seems like the best of both worlds since you'll see only the history you want, and it will remain in order, although you do have to add another flag.

If we go with the patch/review process, I think we should use "git am" as you can do "git am -signoff" to signoff on the patch when you commit it. This will add another line to the comment.

Thanks
Justin
On 2011-03-08, at 12:01 PM, Michelle D'Souza wrote:

> Hi everyone,
> 
> I've come up against the question of what to do with an individual's branch history when responding to a pull request and I'd like to know what other people think. This is not the first time I've had this same question and I'm certain that we'll face it again. 
> 
> The situation arises when a developer works in their own branch for a length of time merging changes from the infusion project repository (upstream) as they work.  In this case, let's look at Golam's recent pull request for 4044. Here's his branch:
> 
> https://github.com/gchowdhury/infusion/tree/FLUID-4044
> 
> In the branch, Golam has done 5 commits interspersed with keeping the branch updated by merging in upstream. If I do a merge of the branch into the upstream master, his commits and merges appear interlaced with the commits that happened in the master. A git log of the repo would show this order:
> 
> commit b94cb72dab13030441b732aad41b67cc164bafb0
> Merge: d800c49 633bc54
> Author: Michelle D'Souza <michelled33 at gmail.com>
> Date:   Tue Mar 8 10:21:00 2011 -0500
> 
>     Merge remote branch 'gchowdhury/FLUID-4044'
> 
> commit d800c49b532c6535b323e0df9d04540a4b390b76
> Author: Justin Obara <obarajustin at gmail.com>
> Date:   Sun Mar 6 17:33:32 2011 -0500
> 
>     FLUID-4127: cleaning up the globals comments
>     
>     I've updated the globals comment at the top of all of our js files, making them consistent. They all have a comment above
> 
> commit 97841c6c3b03cbbc8bccc3192f799756bd266e5a
> Merge: a580593 4151fcc
> Author: Michelle D'Souza <michelled33 at gmail.com>
> Date:   Fri Mar 4 15:33:52 2011 -0500
> 
>     Merge remote branch 'colinbdclark/FLUID-4131'
> 
> commit 4151fcc318aa54dadb4fa39db4959dd86af0b470
> Author: Michelle D'Souza <michelled33 at gmail.com>
> Date:   Fri Mar 4 15:19:24 2011 -0500
> 
>     FLUID-4131: Removing the jquery UI accordion widget from lib.
> 
> commit a58059329c1ddbdc65d90324484a37c37dc86a4e
> Merge: 9d73ac4 01dfa36
> Author: Antranig Basman <antranig.basman at colorado.edu>
> Date:   Fri Mar 4 01:58:00 2011 -0700
> 
>     Merge branch 'FLUID-4045' of git://github.com/harriswong/infusion into harriswong-FLUID-4045
> 
> commit 633bc543132d28afbdfd28667aeb5d36cd262ce7
> Merge: ec19947 9d73ac4
> Author: Golam <golam at golam-desktop.(none)>
> Date:   Thu Mar 3 12:53:54 2011 -0600
> 
>     Merge branch 'master' into FLUID-4044
> 
> commit ec19947b3489a25da71529dfb93061b32c214d4f
> Author: Golam <golam at golam-desktop.(none)>
> Date:   Thu Mar 3 12:43:15 2011 -0600
> 
>     FLUID-4044: cleaned the inline-edit unit test codes with jslint and refactored some more of the codes.
> 
> commit 079038d006eb68d1b8ccd56140dac723cf44c51e
> Author: Colin Clark <colinbdclark at gmail.com>
> Date:   Thu Mar 3 12:13:23 2011 -0500
> 
>     FLUID-4131: Removed jQuery UI accordion widget from UI Options as per new wireframes.
> 
> 
> Notice that commits show in the order they happened in time regardless of which branch they happened on. Exactly what Golam did in each commit will be retained and when he merged with the upstream master will also be retained. This feels to me like too much information. The history of the repo feels like it's being cluttered. 
> 
> In this particular case, if we were still in svn and I was using our patch review process, I would commit two patches - one with Golam's new tests and one with his linting changes. So we are also gaining history here that we wouldn't have had if we were using our old process. I think if I push this upstream the network graph would show a coloured line which branches from the master, has several dots on it and several lines which come down from the master and attach to the coloured line before it goes back up to rejoin the master.   
> 
> I've started to look into the option of using rebase. If I do a rebase using the head of upstream as the new base, I get the following:
> 
> 
> commit 305d5b350a41647a04e46a40fc0c7f13e69ea37f
> Merge: d800c49 16efae3
> Author: Michelle D'Souza <michelled33 at gmail.com>
> Date:   Tue Mar 8 10:50:17 2011 -0500
> 
>     Merge branch 'FLUID-4044'
> 
> commit 16efae39480c0e3191ff3cb3d32b6026fb8ab0f0
> Author: Golam <golam at golam-desktop.(none)>
> Date:   Thu Mar 3 12:43:15 2011 -0600
> 
>     FLUID-4044: cleaned the inline-edit unit test codes with jslint and refactored some more of the codes.
> 
> commit 74d6fa814eefe1168b79a95572616bdc509aead0
> Author: Golam <golam at golam-desktop.(none)>
> Date:   Wed Mar 2 10:48:11 2011 -0600
> 
>     FLUID-4044: cleaned the inline-edit unit test code with jslint and refactored some of the code.
> 
> commit e1b374038b689c59b38ad8cfa49a097a4fb71db2
> Author: Golam <golam at golam-desktop.(none)>
> Date:   Tue Mar 1 13:31:13 2011 -0600
> 
>     FLUID-4044: cleaned the inline-edit unit test code as per code reviewer recomendation.
> 
> commit 0cd8ad10f091af932827cdeee5cd9037ecbc624f
> Author: Golam <golam at golam-desktop.(none)>
> Date:   Fri Feb 25 13:47:43 2011 -0600
> 
>     FLUID-4044: cleaned the inline-edit unit test code as per Jslint recomendation.
> 
> commit 01a87ad130ff058e0964a131b781da05dcfcf015
> Author: Golam <golam at golam-desktop.(none)>
> Date:   Wed Feb 23 10:23:25 2011 -0600
> 
>     FLUID-4044: Conversion of the manual test cases to automated test cases.
> 
> commit d800c49b532c6535b323e0df9d04540a4b390b76
> Author: Justin Obara <obarajustin at gmail.com>
> Date:   Sun Mar 6 17:33:32 2011 -0500
> 
>     FLUID-4127: cleaning up the globals comments
>     
>     I've updated the globals comment at the top of all of our js files, making them consistent. They all have a comment above
> 
> commit 97841c6c3b03cbbc8bccc3192f799756bd266e5a
> Merge: a580593 4151fcc
> Author: Michelle D'Souza <michelled33 at gmail.com>
> Date:   Fri Mar 4 15:33:52 2011 -0500
> 
>     Merge remote branch 'colinbdclark/FLUID-4131'
> 
> 
> 
> Notice that all of Golam's commits and the dates are all retained. What has changed is that they are lumped together and come after the current upstream head. The other thing that has changed is the actual commit hashes. The issue I see here is that I did some code review on github for Golam on particular commits. These commits no longer show up in the history which means that the code review comments will not be connected with upstream.  If I were to commit this I think the network graph would have a coloured line that branches off the master, has several dots on it and then goes back up to connect to the master. Simpler and more clear I think.
> 
> Using an interactive rebase, I can squash some of the commits together with the following result:
> 
> 
> commit 7fb8ab7dfa43fe6dba63bd9b38f0cb7f57157b22
> Merge: d800c49 b3bf014
> Author: Michelle D'Souza <michelled33 at gmail.com>
> Date:   Tue Mar 8 11:10:36 2011 -0500
> 
>     Merge branch 'FLUID-4044'
> 
> commit b3bf0147bf35a06a80a237f524a531b38b2d88b0
> Author: Golam <golam at golam-desktop.(none)>
> Date:   Fri Feb 25 13:47:43 2011 -0600
> 
>     FLUID-4044: cleaned the inline-edit unit test code as per Jslint recomendation.
>     
>     FLUID-4044: cleaned the inline-edit unit test code as per code reviewer recomendation.
>     
>     FLUID-4044: cleaned the inline-edit unit test code with jslint and refactored some of the code.
>     
>     FLUID-4044: cleaned the inline-edit unit test codes with jslint and refactored some more of the codes.
> 
> commit 3e7864fe34d7ac191328c269affbeb01d192ac0b
> Author: Golam <golam at golam-desktop.(none)>
> Date:   Wed Feb 23 10:23:25 2011 -0600
> 
>     FLUID-4044: Conversion of the manual test cases to automated test cases.
> 
> commit d800c49b532c6535b323e0df9d04540a4b390b76
> Author: Justin Obara <obarajustin at gmail.com>
> Date:   Sun Mar 6 17:33:32 2011 -0500
> 
>     FLUID-4127: cleaning up the globals comments
>     
>     I've updated the globals comment at the top of all of our js files, making them consistent. They all have a comment above
> 
> commit 97841c6c3b03cbbc8bccc3192f799756bd266e5a
> Merge: a580593 4151fcc
> Author: Michelle D'Souza <michelled33 at gmail.com>
> Date:   Fri Mar 4 15:33:52 2011 -0500
> 
>     Merge remote branch 'colinbdclark/FLUID-4131'
> 
> 
> Now I've really simplified the history and retained about as much as I would have if I were following the patch review process that I used to work with. I can alter the commit messages if I want to but I wanted to show how I can retain all the commit messages that the person had on their branch. 
> 
> Notice that with the rebasing, the commit hashes on upstream don't change. What has changed is the commit hashes from Golam's branch. 
> 
> My preference is for the last option. I think it leaves us with the cleanest history as well as the simplest network graph. The only downside I see is the loss of code review comments on commits which will not be part of upstream. 
> 
> Thoughts?
> 
> Michelle
> 
> 
> 
> 
> 
> 
> 
> _______________________________________________________
> fluid-work mailing list - fluid-work at fluidproject.org
> To unsubscribe, change settings or access archives,
> see http://fluidproject.org/mailman/listinfo/fluid-work

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://fluidproject.org/pipermail/fluid-work/attachments/20110308/c05bbd32/attachment.html>


More information about the fluid-work mailing list