[cctbxbb] [Dials-support] Massive commit just now

Nicholas Devenish ndevenish at gmail.com
Fri Sep 8 06:57:53 PDT 2017


> In general squashing of commits (turning many related commits into fewer
> commits) should be encouraged before merging.


I enthusiastically agree in practice, but in this case we have a long
lived, multi-month, multi-collaborator project, so rebasing as-you-go is
mostly out for public history reasons.

Rebasing before public merge would take (probably) several hours of
resolving time-ordering problems, and even more time hours to reduce a long
history down to segments of themed commits - (even more to ensure that the
code makes sense and works on each step) - and at some point the time spent
could outweigh the usefulness, especially when it's a core developer
knowingly restructuring large parts of the project. And if you intend to
continue development on the branch after merging parts into master, then
you have to coordinate with everyone collaborating on the public history of
the branch....

I volunteer part of the blame, because I think I underestimated how much
people (other than me) really care about clean git history, and advised
that this would be the quickest way (didn't want to tell someone else to
'spend hours working out conflicts just to make it look neater'). I didn't
check the history properly enough to catch the duplicated commits.

The duplicated commits problem shouldn't have happened and I suspect that
somebody badly rebased along the way - some of the duplicates have James'
as committer when he said he didn't do rebasing - so I wonder if an auto
(pull = rebase) is to blame somewhere. I've never been trusting of this
setting, but it somewhat allows ignoring the more complicated parts of
git's model (until it goes wrong of course, which describes most of git's
abstractions).

A potential rule of thumb for this could be: If the commit is so small as
> to not have a helpful commit message ('bugfix', 'typo', '... part 1', '...
> part 2') then it should be squashed together with closely related other
> commits. Would mean when we are reviewing this in a year to work out where
> something went wrong, it's easier to track down the important commits....


Definitely if deliberately tidying for a public announcement/pull request,
but too much of this "as you go" just makes more work for anybody else
working on the branch at the same time.

I personally prefer merging feature branches (of more than a few commits)
via split & merge, with rebase if possible e.g.

  git checkout <feature>
  git rebase master
  git co master
  git merge --no-ff <feature> -m "Merge <description of what feature branch
does>"

Because then the entire set of feature commits are on top of master, but
the commits are still identifiable as belonging together and separate,
looking something like this:

--o----------------o    master
   \--o--o--o--o--/     <feature>

Pull requesting allows you to punt the decision to somebody else, of course.

I guess it comes down to how much time you spend "polishing" the branch
before commit, and pull requests encourage polishing because it comes under
more scrutiny (whereas otherwise it might not have so many objections on
the basis of 'not looking neat').

Nick
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://phenix-online.org/pipermail/cctbxbb/attachments/20170908/50f2aa45/attachment.htm>


More information about the cctbxbb mailing list