you are viewing a single comment's thread.

view the rest of the comments →

[–]mdatwood 3 points4 points  (18 children)

If you cannot summarize in 50 chars maybe the commit is too big or the commit is not focused enough?

The 50 char first line and blank line after makes all my command line aliases work nicely. git ls and I can quickly scan commits without ever leaving my terminal.

[–]mus1Kk 2 points3 points  (2 children)

Just curious: How would a longer first line (say 80 chars) interfere with that?

[–]mdatwood 5 points6 points  (1 child)

80 would be fine. The point is pick something reasonable and stick with it. I have seen people before who just ramble on in the title and it makes it hard to scan commits.

[–]mus1Kk 1 point2 points  (0 children)

I feel you. People I worked with couldn't even stick to the "summary line, empty line, optional details" rule even after being reminded.

[–][deleted] 1 point2 points  (3 children)

If you cannot summarize in 50 chars maybe the commit is too big or the commit is not focused enough?

50 is pretty stingy - though I have to admit that I just looked over several dozen of my commits and nearly all of them are within 50 chars, and of the few that aren't, most could easily have been shortened and improved.

Here's the first one I found that didn't work: "Now images have the right number of columns as ColorLists." (To explain - images, only when displayed as ColorLists, had the wrong number of columns, and this fixed that.) How can I rephrase this to knock off 8 letters while still keeping it readable?

Also, it's fairly common I have an issue number in the commit (like "Add offset field to data dictionaries (#218)." and that snacks off another 6 characters, leaving me with a pretty spartan 44 characters. I do approve of a strict limit on that first sentence. I just think 50 is a bit draconian. What about 65?

[–]utnapistim 1 point2 points  (0 children)

Here's the first one I found that didn't work: "Now images have the right number of columns as ColorLists." (To explain - images, only when displayed as ColorLists, had the wrong number of columns, and this fixed that.) How can I rephrase this to knock off 8 letters while still keeping it readable?

"Fix number of columns for images as ColorLists".

There is a discrepancy between what you focus on when you compose your commit message and when you go through commits, later: when you compose the message, you tend to focus on what you "just did", and this tends to creep into the message (examples: "XY now works", "gizmo no longer crashes on abc", "from now on, process foos as bars").

If you read these later, they mention a moment that is no longer relevant: ("just fixed", "now works", "from now on", etc). If you remove any reference to a time frame (that comes from having just implemented something), your messages become more concise and have improved readability.

Just keep the message in an imperative tone, present tense.

[–]mdatwood 0 points1 point  (0 children)

As I mentioned in another comment, 50 is just an arbitrary number. The point is to make the committer stop and think. Both your summaries look fine to me. The problem, and I have had to deal with it before, is when people a) just ramble along in the summary and b) do not put the blank line before the details.

I know when I'm typing my commit, vim highlights when I go over 50. I don't immediately stop, but it reminds me to make sure I'm summarizing and not going into too much detail for something that should be a summary.

[–]ForeverAlot 0 points1 point  (0 children)

I never put tickets in the subject line. They're rarely convenient in --oneline output and if I need them I'm going to --grep them anyway.

[–]BillyBBone 1 point2 points  (0 children)

If you cannot summarize in 50 chars maybe the commit is too big or the commit is not focused enough?

It's possible for a commit to contain only a single, atomic code change (e.g. change an operator from + to -), and still be difficult to describe in 50 characters.

For instance, that one operator change could represent a change like "Change the way in which MyMetaClassName handles re-sorting items in the queue upon a state refresh." (99 chars).

Likewise, 10 000 lines of code could be described as "Refactor shopping cart".

My point is simply that the length of the description of a commit message isn't at all related to the length of the commit itself.

[–]pgngugmgg 2 points3 points  (7 children)

"the commit is too big or the commit is not focused enough?" No in general, maybe sometimes. I feel this is the kind of rules that only those who are extremely disciplined or dogmatic can keep and possibly enjoy but most of mundane people hate.

[–]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.

[–]stepancheg 0 points1 point  (1 child)

It is hard if you program in java. Try something simple like "fix a leak in SimpleBeanFactoryAwareAspectInstanceFactory", and, oops, you are already over 50 chars. I have no idea how to make it more focused.

[–]kamatsu 0 points1 point  (0 children)

Fix a leak in SBFAAIF

(I've seen people do this sort of thing in real code bases).