Folks, Earlier a branch which was fairly long lived was merged in perhaps a non-ideal way, apparently replaying every commit from around July to now. While this is confusing and has (a little) messed with history, it has not actually broken anything... However, if you have a big branch you want to merge, I think a pull request makes this a little more obvious Just some background to what happened... Cheers Graeme -- This e-mail and any attachments may contain confidential, copyright and or privileged material, and are for the use of the intended addressee only. If you are not the intended addressee or an authorised recipient of the addressee please notify us of receipt by returning the e-mail and do not use, copy, retain, distribute or disclose the information in or attached to the e-mail. Any opinions expressed within this e-mail are those of the individual and not necessarily of Diamond Light Source Ltd. Diamond Light Source Ltd. cannot guarantee that this e-mail or any attachments are free from viruses and we cannot accept liability for any damage which you may sustain as a result of software viruses which may be transmitted in or with the message. Diamond Light Source Limited (company no. 4375679). Registered in England and Wales with its registered office at Diamond House, Harwell Science and Innovation Campus, Didcot, Oxfordshire, OX11 0DE, United Kingdom
Thinking some more about this, maybe it would be good practice in the future for large merges, say 10 or more commits, and merges from long-standing branches to create a pull request, and leave that for a couple of working days so that other people can have a look first and comment on how to proceed. In general squashing of commits (turning many related commits into fewer commits) should be encouraged before merging. I appreciate here that James had asked people to check out the branch which I had, and run the tests... 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.... What do people think? Thanks Graeme -- This e-mail and any attachments may contain confidential, copyright and or privileged material, and are for the use of the intended addressee only. If you are not the intended addressee or an authorised recipient of the addressee please notify us of receipt by returning the e-mail and do not use, copy, retain, distribute or disclose the information in or attached to the e-mail. Any opinions expressed within this e-mail are those of the individual and not necessarily of Diamond Light Source Ltd. Diamond Light Source Ltd. cannot guarantee that this e-mail or any attachments are free from viruses and we cannot accept liability for any damage which you may sustain as a result of software viruses which may be transmitted in or with the message. Diamond Light Source Limited (company no. 4375679). Registered in England and Wales with its registered office at Diamond House, Harwell Science and Innovation Campus, Didcot, Oxfordshire, OX11 0DE, United Kingdom
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
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
participants (3)
-
Aaron Brewster -
Graeme.Winter＠diamond.ac.uk -
Nicholas Devenish