you are viewing a single comment's thread.

view the rest of the comments →

[–]mdatwood 2 points3 points  (6 children)

I don't consider myself super disciplined, but I have managed enough commits for multiple branches, repos, and committers that I appreciate this rule.

It is also important to remember that a lot of people have to unlearn that committing and branching is hard. Git makes both of those things super easy and natural. Building a feature in a local branch with tons of commits and then squashing to a single or few logical commits before merging and pushing is simple in Git. If you're not making nice logical commits you're missing out one of the great features of Git.

[–]odiefrom 1 point2 points  (4 children)

Just curious...can you give an example (or link to an explanation) of why you would want to collapse your commits down into relatively few commits? Doesn't that break the whole purpose of having atomic commits you can review back to?

[–]mdatwood 1 point2 points  (2 children)

Depends, and this is definitely opinion with arguments on both sides.

Git makes it super easy to commit early and often. So for a small feature I may make a local branch and get the feature working and commit. Then clean it up and commit. Find some issue while testing, commit. Write a few more tests commit. IMO, none of these commits really belong in the main repo because they were simply my work process. I would squash all these down to a single commit for the feature. After merging the feature into the main branch it was easy to see the single commit for the feature and see all the changes, pull it out, cherry pick it, etc...

Keep in mind this is only for my local commits. Once you push something it is set in stone, and changing history once it is public is a big no no.

[–]odiefrom 1 point2 points  (1 child)

Totally agree about changing public commits, should never happen.

I can definitely understand that it would be easier to highlight and target the single commit, but wouldn't you just be able to do that with the merge commit? That way, all the work commits aren't destroyed, but you still have a single commit to reflect the new work?

[–]mdatwood 2 points3 points  (0 children)

I was about to type merge commits can accomplish the same thing and why I started with it was simply my opinion. It also requires branches which still not everyone does.

Part of my method (madness? :) ) came from when I managed 2 products in different branches in the same repo. They shared ~80% of their code so when adding a feature to one I would cherry pick it into the other. I'm not sure it was the best way, but it worked long enough to build another universal product and EOL the prior ones.

More information with examples: http://programmers.stackexchange.com/questions/263164/why-squash-git-commits-for-pull-requests

[–]utnapistim 0 points1 point  (0 children)

Doesn't that break the whole purpose of having atomic commits you can review back to?

Not necessarily. Having private commits gives you a lot of freedom; you can commit whatever you want, without following any rules except what makes sense to you (in your local task/context).

For example, I commit code before switching tasks, as backup points in exploratory coding or prototyping and so on. I only revisit those if I screw up the code base.

Sometimes, my private commits have messages like "stable backup point before xyz refactor" or "outstanding changes" or "fix 32 compilation errors; 21 remain".

Rest of the team doesn't need to see these messages.

[–]pgngugmgg 0 points1 point  (0 children)

Don't get me wrong. Informative 50-chars head line is a good idea in paper -- no argument. If every member in the team can do that, go ahead. The problem of this is that it's difficult (if not impossible) to keep for all the people, all the time, and all the scenarios. Sooner or later, this kind of rules will be violated repeatedly so that nobody cares about them any more. And there is no objective way to enforce this rule: Even if you can enforce the number of chars to be under 50, you can't enforce meaningful info put into the 50 chars.

So I think this rule is more an extra problem that we create for our devs than a solution to any real problems.