openstack team mailing list archive

[Mark McLoughlin <markmc@xxxxxxxxxx>]

Hi Vish,

On Fri, 2011-09-23 at 10:04 -0700, Vishvananda Ishaya wrote:

> 2) Sorry about the review spam earlier.  I was trying to bring in an
> old branch and keep the commit history.  It seems that in the new
> world of gerrit, we're going to end up squashing everything in to one
> commit anyway so that is not the right way to go.

There's definitely a shift in how to think about commits and merges etc.
with git, but I wouldn't quite put as "squash everything into one
commit".

I haven't used launchpad/bzr much, but it seemed like with bzr merge
props we ended up with a single patch to review but the history of how
that patch was arrived at.

With git, merge requests can have multiple, individually reviewable
logical, bisectable commits on the proposed branch but no history of the
revisions the author went through with each of those patches.

As a simple example, this branch in bzr:

  https://code.launchpad.net/~markmc/nova/iscsi-tgtadm-choice/+merge/75906

had 7 commits. A commit by Chuck, three fixes to that commit, a trunk
merge by me and two more commits by me. The reviewer can look at the
individual commits, but I suspect most just review it as one big patch.

With git, the branch:

  https://review.openstack.org/#q,status:open+project:openstack/nova+branch:master+topic:bug/819997,n,z

has 3 commits. The original patch from Chuck including fixes made to it
later and my two patches. This way the multiple contributors are still
getting credit, but the change is more easily reviewable because each
logically separate change is in its own commit.

So, I guess the takeaways are:

  1) Each commit you push is going to be tested by jenkins, so you need 
     to make sure the tests pass for one

  2) You need to squash fixes for your commits back into the 
     appropriate commit

  3) But you should attempt to keep logically separate changes[1] as 
     individual commits for the sake of easier review and bisection.

Cheers,
Mark.

[1] - best tip I've heard on this is to always keep in mind what your
next commit message is going to be. If it will be "Refactor foo, fix
bar, add blaa", then you should probably have separate commits.