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
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 [email protected] https://lists.sourceforge.net/lists/listinfo/dials-support