11 ms·

A Note About Git Commit Messages

104 points by sant0sk1 16y ago

e40 16y ago
This may be a mundane post, but it's sorely needed IMO. Way too many people create crappy commit messages. I've started kicking them back to the developer to fix.
- draebek 16y ago
  How do you ask them to fix them? I'm imagining that you see the commit message after they've pushed it up somewhere. Are you asking them to reword/amend the commit--i.e., change the history of a pushed branch, with the little bit of extra work that involves?
  - js2 16y ago
    You do the review before the commit is pushed to the shared public branch. Lots of ways to do this. Here's three: 1. Use format-patch and review via email. 2. git push origin +HEAD:cr/js2 Now another developer can pull that branch and review it before it is merged to master. 3. Use Gerrit.
  - Xurinos 16y ago
    This isn't really an issue. Contrary to what a lot of people will tell you, it is fine to change the history of a pushed branch -- especially for the most recent commit -- so long as other developers adopt a policy of rebasing after fetch. Here is a concrete example: (bare repo): # Make sure receive.denyDeleteCurrent = ignore (Dev A): git push (Dev B): git pull --rebase # you can make this the default for a particular tracking branch so it is just "git pull" (Dev B): # do some work (Dev B): git commit (Dev A): git commit --amend (Dev A): git push origin :master # Deletes remote master. Not required for shared branches -- those can simply be replaced -- but "master" is usually the "current branch" for a repo, so it bears knowing. (Dev A): git push origin master:master # And now we replace the remote's version with our local version. (Dev B): git pull --rebase # Updates pointers and applies the new commit on top. Yes, if you do even more devastating history changes than this, Dev B needs to dig a little further on why his rebase is going haywire. THIS is why people fear the history change; it is sometimes extra work for downstream developers to handle the rearrangements (and we developers like to be lazy and for everything to work smoothly the first time). Thankfully, downstream developers can solve these problems with a few git tools, including "rebase --onto" and cherry-picking. But in the case of a quick and simple commit amend, all is well. Alternatively, you can code review, but sometimes people are in a rush, and you run into this situation. In most environments, I wager it will be okay, especially if there is communication on the changes and reasonable education on version control theory.
    - draebek 16y ago
      Interesting, thanks for the example, works exactly as you said. There is a race when dev A deletes master though, right? What if someone else made a commit between when dev A last pulled and when dev A deletes master, wouldn't that commit be lost? In the process of testing this I managed to lose dev B's work a couple times, so I still think this kind of operation is too risky to be worthwhile. In case anyone is interested I made a test script based on Xurinos's example: https://gist.github.com/769904 https://gist.github.com/769904
      - Xurinos 16y ago
        There is a race when dev A deletes master though, right? Yes and no. I could say that this is where communication can help a little, and it depends entirely on your local group's policies, but that would be the easy cop-out. Let's say it really happened. Dev B's git pull --rebase will also lose the change; it was assumed to be pushed up, and a git fetch means that you are reflecting exactly what is in the remote branch. But changes are never truly lost+++. Dev B can check his reflog, find his original commit, and cherry-pick it in, resubmitting it. Afterwards, he can bonk Dev A over the head for not pulling before pushing. There is a danger that Dev B would not notice, but from a little test I just did, he is going to get some nasty messages during his git pull --rebase that should indicate that something interesting just happened. I have not looked into this, but does anyone know if it is possible to lock a remote git repo (aside from the automatic push lock)? If one could manually lock the repo, the process would be to lock, pull, push-delete, push-for-real, and unlock. +++ depending on your git configuration policy. I think you have 30 days before dangling commits are truly pruned permanently.
        draebek 16y ago
        > I have not looked into this, but does anyone know if it is possible to lock a remote git repo (aside from the automatic push lock)? If one could manually lock the repo, the process would be to lock, pull, push-delete, push-for-real, and unlock. Or maybe if Git could do a "test and set" on a ref? For example, instead of two step delete and recreate, how about "git push origin +master": forced update, but your Git sends "update master to 1234abcd iff your current master is 5678fedc" where 5678fedc is where origin/master points on your local Git. If someone had snuck in a new commit Git would then fail and you could deal with it. (Of course, on a busy shared repository "deal with it" could take a while if people keep sneaking in commits; but I suspect that kind of commit volume is very rare.)
  - e40 16y ago
    They push to an intermediate repo, where I can look at the changes. For some projects, we use gerrit, which makes this really easy (it's a review system from google).
telemachos 16y ago
And in case anyone needs negative examples (or a good laugh), check out http://whatthecommit.com/ http://whatthecommit.com/
198d 16y ago
Ever since reading that post, I've tried my best to adhere to similar guidelines. On a personal level, it kind of gives you a little 'retro' on what is was you just did, which is never a bad thing. I've also found myself making larger, more meaningful commits, which, I think, is a good thing.
- barclay 16y ago
  Agreed. I've found that for me, to have meaningful commit messages, you have to have a meaningful amount of work complete. Otherwise you end up with a bunch of "more work on feature x" messages. This is counter to a lot of the "commit often" mantra that so many git people have, but it's working better for me.
  - Xurinos 16y ago
    "commit often" is fine. It's "push often" that gets you into trouble. With an interactive rebase, you can polish up your commits and their messages before pushing them, presenting to your peers a finely crafted (readable, reviewable, maintainable) publication.
    - wladimir 16y ago
      Indeed, I only discovered that recently and it makes git pretty cool. It has the added advantage of being able to merge commits that fix intermediate bugs that you introduced yourself and fixed immediately. There is no reason to bore other people with those.
  - jerf 16y ago
    Commit often, fix them up. This is why I just can't side with the people who claim rebasing and rewriting your personal history is some sort of affront against nature. Take their argument and add it to the need for useful commits and what it unpacks to is that you should forgo all use of your source control until you have created a Perfect Patch, at which you are finally allowed to commit, until you create the next Perfect Patch. Of course that's not how they see it but it is what it actually translates to, inescapably. Really misses the point of the whole thing if you ask me; it really feels like accidentally raising old hacks to best practices.
davidcuddeback 16y ago
I find it unnatural to write commit messages in the present tense. To me, it seems like commit messages are a log of what work has been done, which would naturally be described in past tense. Nonetheless, I do write my commit logs in present tense for the sake of consistency. Does anyone have a good explanation of why commit messages should be in the present tense? (Beyond what the article says: "This convention matches up with commit messages generated by commands like git merge and git revert.") Why do commands like git merge and git revert generate commit messages in the present tense? The best explanation that I can think of is an analogy to literature: literary analysis is written in the present tense, because any time you open a book, the story is happening now, in the present tense. I don't quite buy that, but it's the best explanation I've come up with.
- tedunangst 16y ago
  A patch that changes X to Y will always change X to Y. Maybe you applied the patch in the past, but the patch itself will never stop changing X to Y.
  - aerique 16y ago
    You win the aerique Daily HN award for the best and most concise answer. From now on I will write my commit messages in the present tense.
- shasta 16y ago
  My guess is that it's because it was designed from the point of view of someone looking at patches to be added, rather than from the point of view of someone who writes a bunch of code. Imagine you're looking at a patch and asking what it does. It's more natural to describe that this patch will "add support for feature X" than "added support for feature X".
  - kleiba 16y ago
    When the message is supposed to describe what the patch does, shouldn't it then be written in the 3rd person singular: "adds support for feature X"?
yannk 16y ago
The recent-ish vim syntax file for git commit message takes those recommendations into account - they don't do anything about the present vs. past tense obviously, but come up with color warnings, which helps a lot. That's actually how I learned about those preferences and the reasoning behind in the first place btw.
- telemachos 16y ago
  I believe that the syntax file is the work of Tim Pope - the author of the blog post we're talking about. http://vim.runpaint.org/extending/integrating-vim-with-git/ http://vim.runpaint.org/extending/integrating-vim-with-git/
brown9-2 16y ago
I consider "ability to write a good SCM commit message" one of the boxes to check on my "Is this person a good developer?" checklist.
nailer 16y ago
> wrapping your commit messages to 72 columns > On an 80 column terminal > Good email netiquette dictates we wrap our plain text emails (for) an 80 column terminal. I think format should be separated from presentation and presuming people should use a particular display device is wrong. These are well established principles and Unix, although lacking a tradition of interface design, isn't some kind of exception. * I use a modern email program (I use gmail but this would include, say, mutt). * I sometimes use a wider terminal. Sometimes I use a narrower one. On my system (OS X default 10.6) less handles this just fine unless some 72-character-neckbeard screws it up. * I sometimes use a GUI tool to examine messages, like github. These may be wider or narrower. In all these cases forced 72 character wrappings look terrible.
- steveklabnik 16y ago
  Exactly. That's why you write it to the lowest common denominator, so that everyone can use it well.
  - nailer 16y ago
    Agreed: re: making sure that everyone can use it well, but targeting LCD will break newer devices sometimes. As far as I can tell, using linebreaks for linebreaks will give you readable output on the widest variety of devices.
  - lallysingh 16y ago
    And let's not forget where else where the text may end up later. I'm sure this message was fine in their mail reader: http://www.ietf.org/mail-archive/web/manet/current/msg11520.html http://www.ietf.org/mail-archive/web/manet/current/msg11520.... (chosen only b/c I was reading through the list this morning) But now I must horizontally scroll :(
    - uxp 16y ago
      To be fair, thats a problem with the way browsers render anything inside the <pre> tag. If anything, it is an example of what people should do, not force unnecessary restrictions on the way they write text. Lest you might end up with: > This is a short paragraph that I am quoting that > someone > else sent to me.
binspace 16y ago
72 character lines is unnatural to read for long sentences (especially in web browsers). Also, less has line wrapping and who develops using 80 character terminals anymore? Maybe the author has a time machine back to 1981? I want access to that!
- dedward 16y ago
  A lot of people still prefer work with revision control tools from a standard unix command line.... in an 80x25-ish window.....especially when it comes to debugging and investigating things (so we can use all our other nice shell text processing tools to make sense of things) Something that fits nicely in that space is helpful in that case.
mikeklaas 16y ago
My favourite commit message this week: "improve crapping" (typo: should be cropping)

104 points by sant0sk1 16y ago

e40 16y ago

This may be a mundane post, but it's sorely needed IMO. Way too many people create crappy commit messages. I've started kicking them back to the developer to fix.

draebek 16y ago
How do you ask them to fix them? I'm imagining that you see the commit message after they've pushed it up somewhere. Are you asking them to reword/amend the commit--i.e., change the history of a pushed branch, with the little bit of extra work that involves?
- js2 16y ago
  You do the review before the commit is pushed to the shared public branch. Lots of ways to do this. Here's three: 1. Use format-patch and review via email. 2. git push origin +HEAD:cr/js2 Now another developer can pull that branch and review it before it is merged to master. 3. Use Gerrit.
- Xurinos 16y ago
  This isn't really an issue. Contrary to what a lot of people will tell you, it is fine to change the history of a pushed branch -- especially for the most recent commit -- so long as other developers adopt a policy of rebasing after fetch. Here is a concrete example: (bare repo): # Make sure receive.denyDeleteCurrent = ignore (Dev A): git push (Dev B): git pull --rebase # you can make this the default for a particular tracking branch so it is just "git pull" (Dev B): # do some work (Dev B): git commit (Dev A): git commit --amend (Dev A): git push origin :master # Deletes remote master. Not required for shared branches -- those can simply be replaced -- but "master" is usually the "current branch" for a repo, so it bears knowing. (Dev A): git push origin master:master # And now we replace the remote's version with our local version. (Dev B): git pull --rebase # Updates pointers and applies the new commit on top. Yes, if you do even more devastating history changes than this, Dev B needs to dig a little further on why his rebase is going haywire. THIS is why people fear the history change; it is sometimes extra work for downstream developers to handle the rearrangements (and we developers like to be lazy and for everything to work smoothly the first time). Thankfully, downstream developers can solve these problems with a few git tools, including "rebase --onto" and cherry-picking. But in the case of a quick and simple commit amend, all is well. Alternatively, you can code review, but sometimes people are in a rush, and you run into this situation. In most environments, I wager it will be okay, especially if there is communication on the changes and reasonable education on version control theory.
  - draebek 16y ago
    Interesting, thanks for the example, works exactly as you said. There is a race when dev A deletes master though, right? What if someone else made a commit between when dev A last pulled and when dev A deletes master, wouldn't that commit be lost? In the process of testing this I managed to lose dev B's work a couple times, so I still think this kind of operation is too risky to be worthwhile. In case anyone is interested I made a test script based on Xurinos's example: https://gist.github.com/769904 https://gist.github.com/769904
    - Xurinos 16y ago
      There is a race when dev A deletes master though, right? Yes and no. I could say that this is where communication can help a little, and it depends entirely on your local group's policies, but that would be the easy cop-out. Let's say it really happened. Dev B's git pull --rebase will also lose the change; it was assumed to be pushed up, and a git fetch means that you are reflecting exactly what is in the remote branch. But changes are never truly lost+++. Dev B can check his reflog, find his original commit, and cherry-pick it in, resubmitting it. Afterwards, he can bonk Dev A over the head for not pulling before pushing. There is a danger that Dev B would not notice, but from a little test I just did, he is going to get some nasty messages during his git pull --rebase that should indicate that something interesting just happened. I have not looked into this, but does anyone know if it is possible to lock a remote git repo (aside from the automatic push lock)? If one could manually lock the repo, the process would be to lock, pull, push-delete, push-for-real, and unlock. +++ depending on your git configuration policy. I think you have 30 days before dangling commits are truly pruned permanently.
      - draebek 16y ago
        > I have not looked into this, but does anyone know if it is possible to lock a remote git repo (aside from the automatic push lock)? If one could manually lock the repo, the process would be to lock, pull, push-delete, push-for-real, and unlock. Or maybe if Git could do a "test and set" on a ref? For example, instead of two step delete and recreate, how about "git push origin +master": forced update, but your Git sends "update master to 1234abcd iff your current master is 5678fedc" where 5678fedc is where origin/master points on your local Git. If someone had snuck in a new commit Git would then fail and you could deal with it. (Of course, on a busy shared repository "deal with it" could take a while if people keep sneaking in commits; but I suspect that kind of commit volume is very rare.)
- e40 16y ago
  They push to an intermediate repo, where I can look at the changes. For some projects, we use gerrit, which makes this really easy (it's a review system from google).

telemachos 16y ago

And in case anyone needs negative examples (or a good laugh), check out http://whatthecommit.com/ http://whatthecommit.com/

198d 16y ago

Ever since reading that post, I've tried my best to adhere to similar guidelines. On a personal level, it kind of gives you a little 'retro' on what is was you just did, which is never a bad thing. I've also found myself making larger, more meaningful commits, which, I think, is a good thing.

barclay 16y ago
Agreed. I've found that for me, to have meaningful commit messages, you have to have a meaningful amount of work complete. Otherwise you end up with a bunch of "more work on feature x" messages. This is counter to a lot of the "commit often" mantra that so many git people have, but it's working better for me.
- Xurinos 16y ago
  "commit often" is fine. It's "push often" that gets you into trouble. With an interactive rebase, you can polish up your commits and their messages before pushing them, presenting to your peers a finely crafted (readable, reviewable, maintainable) publication.
  - wladimir 16y ago
    Indeed, I only discovered that recently and it makes git pretty cool. It has the added advantage of being able to merge commits that fix intermediate bugs that you introduced yourself and fixed immediately. There is no reason to bore other people with those.
- jerf 16y ago
  Commit often, fix them up. This is why I just can't side with the people who claim rebasing and rewriting your personal history is some sort of affront against nature. Take their argument and add it to the need for useful commits and what it unpacks to is that you should forgo all use of your source control until you have created a Perfect Patch, at which you are finally allowed to commit, until you create the next Perfect Patch. Of course that's not how they see it but it is what it actually translates to, inescapably. Really misses the point of the whole thing if you ask me; it really feels like accidentally raising old hacks to best practices.

davidcuddeback 16y ago

I find it unnatural to write commit messages in the present tense. To me, it seems like commit messages are a log of what work has been done, which would naturally be described in past tense. Nonetheless, I do write my commit logs in present tense for the sake of consistency. Does anyone have a good explanation of why commit messages should be in the present tense? (Beyond what the article says: "This convention matches up with commit messages generated by commands like git merge and git revert.") Why do commands like git merge and git revert generate commit messages in the present tense? The best explanation that I can think of is an analogy to literature: literary analysis is written in the present tense, because any time you open a book, the story is happening now, in the present tense. I don't quite buy that, but it's the best explanation I've come up with.

tedunangst 16y ago
A patch that changes X to Y will always change X to Y. Maybe you applied the patch in the past, but the patch itself will never stop changing X to Y.
- aerique 16y ago
  You win the aerique Daily HN award for the best and most concise answer. From now on I will write my commit messages in the present tense.
shasta 16y ago
My guess is that it's because it was designed from the point of view of someone looking at patches to be added, rather than from the point of view of someone who writes a bunch of code. Imagine you're looking at a patch and asking what it does. It's more natural to describe that this patch will "add support for feature X" than "added support for feature X".
- kleiba 16y ago
  When the message is supposed to describe what the patch does, shouldn't it then be written in the 3rd person singular: "adds support for feature X"?

yannk 16y ago

The recent-ish vim syntax file for git commit message takes those recommendations into account - they don't do anything about the present vs. past tense obviously, but come up with color warnings, which helps a lot. That's actually how I learned about those preferences and the reasoning behind in the first place btw.

telemachos 16y ago
I believe that the syntax file is the work of Tim Pope - the author of the blog post we're talking about. http://vim.runpaint.org/extending/integrating-vim-with-git/ http://vim.runpaint.org/extending/integrating-vim-with-git/

brown9-2 16y ago

I consider "ability to write a good SCM commit message" one of the boxes to check on my "Is this person a good developer?" checklist.

nailer 16y ago

> wrapping your commit messages to 72 columns > On an 80 column terminal > Good email netiquette dictates we wrap our plain text emails (for) an 80 column terminal. I think format should be separated from presentation and presuming people should use a particular display device is wrong. These are well established principles and Unix, although lacking a tradition of interface design, isn't some kind of exception. * I use a modern email program (I use gmail but this would include, say, mutt). * I sometimes use a wider terminal. Sometimes I use a narrower one. On my system (OS X default 10.6) less handles this just fine unless some 72-character-neckbeard screws it up. * I sometimes use a GUI tool to examine messages, like github. These may be wider or narrower. In all these cases forced 72 character wrappings look terrible.

steveklabnik 16y ago
Exactly. That's why you write it to the lowest common denominator, so that everyone can use it well.
- nailer 16y ago
  Agreed: re: making sure that everyone can use it well, but targeting LCD will break newer devices sometimes. As far as I can tell, using linebreaks for linebreaks will give you readable output on the widest variety of devices.
- lallysingh 16y ago
  And let's not forget where else where the text may end up later. I'm sure this message was fine in their mail reader: http://www.ietf.org/mail-archive/web/manet/current/msg11520.html http://www.ietf.org/mail-archive/web/manet/current/msg11520.... (chosen only b/c I was reading through the list this morning) But now I must horizontally scroll :(
  - uxp 16y ago
    To be fair, thats a problem with the way browsers render anything inside the <pre> tag. If anything, it is an example of what people should do, not force unnecessary restrictions on the way they write text. Lest you might end up with: > This is a short paragraph that I am quoting that > someone > else sent to me.

binspace 16y ago

72 character lines is unnatural to read for long sentences (especially in web browsers). Also, less has line wrapping and who develops using 80 character terminals anymore? Maybe the author has a time machine back to 1981? I want access to that!

dedward 16y ago
A lot of people still prefer work with revision control tools from a standard unix command line.... in an 80x25-ish window.....especially when it comes to debugging and investigating things (so we can use all our other nice shell text processing tools to make sense of things) Something that fits nicely in that space is helpful in that case.

mikeklaas 16y ago

My favourite commit message this week: "improve crapping" (typo: should be cropping)