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

[Aaron Brewster]

Hi folks, I agree about using pull requests.  They really help to catch
problems like duplicated commits since all the commits about to be added
are listed and can be reviewed.

I worry about overuse of commit squashing.  Small commits like 'bugfix' and
'typo' can be squashed I suppose, but taking 20 commits that represent
weeks or more of work and compressing them to a single commit obfuscates
the history.  Detailed commit messages ensure a solid history.  Squashed
commits make that history harder to track from what I've seen.

I'm happy to read a lot of commit messages.  It doesn't take long.  I think
it's important to watch what other developers are doing.

-Aaron

On Fri, Sep 8, 2017 at 6:57 AM, Nicholas Devenish <ndevenish at gmail.com>
wrote:

>
> 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
>
>
> ------------------------------------------------------------
> ------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> _______________________________________________
> DIALS-support mailing list
> DIALS-support at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/dials-support
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://phenix-online.org/pipermail/cctbxbb/attachments/20170908/ffa7526d/attachment-0001.htm>