6 ms·
Rob Pike on good commit messages (2014)
- caiocaiocaio 7y agoOne one hand, I like a nice long commit message. On the other hand, human nature is terrible. I know that if long commit messages catch on, some synergetic managers will start insisting on minimum lengths for commit messages, and start having meetings discussing standards and policies for commit messages, and start filling binders with more rules than one person could ever memorize. Soon, fixing a single-character typo in a comment will take 20 minutes, and pressing enter will be nerve-racking because you know there's just something you're forgetting from the binder.
- safog 7y agoI feel you here, but it's up to tech leads / Sr. Engineers to push back on management bullshit like this because otherwise the commit messages aren't the only thing in that culture that'll make an engineer miserable. My management horror story here is having a SCM (source code manager) who was responsible for taking code checked into a VCS and deploying it to prod (using a CI system no less). He used to insist on engineers sending him a word document with a very specific template on what files changed, what binaries need to be re-deployed etc. etc. for every single commit or it wouldn't get deployed at all and his day was made up of yelling at engineers because the template was incorrectly formatted, or used the wrong font or whatever. Noone else could touch prod (or even staging) except him and all of this process originated from an engineer messing up a deployment and bringing down prod. My takeaway from this entire experience was tech middle management at smaller, enterprise-y shops was just completely broken and all these guys were capable of was playing silly political games. If possible, just work with smart people, if you can't evaluate if someone's smart during an interview process, just work for one of the big tech-cos and you can't go wrong.
- hprotagonist 7y agoAt about the same time, Chris Beams wrote this, which I have loved ever since: https://chris.beams.io/posts/git-commit/ https://chris.beams.io/posts/git-commit/ I note with some glee that magit colorizes my commit text and flags long lines,etc. largely in the style of this advice. The money: The seven rules of a great Git commit message Keep in mind: This has all been said before. Separate subject from body with a blank line Limit the subject line to 50 characters Capitalize the subject line Do not end the subject line with a period Use the imperative mood in the subject line Wrap the body at 72 characters Use the body to explain what and why vs. how*
- nogabebop23 7y agoI shared a variation of this with our development managers (consciously changing "rules" to "tips & suggestions" so as not to ignite a tabs v. spaces holy war) and got very strong responses about how "as long as you get a ummary with some context and cosistency, and the issue # linked this is overkill". Okay, well how do we enure that we get this without some guidance? It's very frustrating.
- blowski 7y agoDo the team already have a consensus? Do they believe there’s a problem?
- metalliqaz 7y agobut that is almost entirely about formatting and cosmetics. I don't give a flying fart about line wrapping if the comment adequately explains the reasoning behind a change.
- RussianCow 7y ago> Limit the subject line to 50 characters It's nearly impossible to get certain points across in 50 characters. I keep mine under 72, and even then, I sometimes struggle to adequately describe a change in that little space, even at a high level, to the point that it would be useful for someone searching for something. Honestly, all of this stuff matters way less than the actual content of the commit message anyway. Style is worthless if there is no substance.
- saagarjha 7y ago> It's nearly impossible to get certain points across in 50 characters. Usually that kind of stuff goes in the body of the commit message.
- RussianCow 7y agoBut most Git UIs only show the first line of the commit message when displaying the log, so if you're searching for something specific, you're only going to see that without clicking into a commit. I want that first line to tell me whether this commit is relevant to whatever I'm looking for, and it's extremely difficult to do that in 50 characters.
- deanmoriarty 7y agoAfter being in the field for almost a decade now and guilty of so many poor commit messages, I came to the firm realization that a very important trait of a good software engineer is the ability and diligence to write detailed commit messages, and I would have never remotely imagined this during the first few years of my career (I was "raised" in companies who didn't care at all about this and all commits were always "add x", "fix foo", "fix build", ...). It became way too frustrating when I looked back at code committed by myself (or others) years prior and wondered "what is the reason why this code exists?" These days, every single commit I push has an entire paragraph or two clearly describing the changes, and the motivation behind them, even if it's a one-line diff. My team praises me for this, and I think I've been able to influence the culture as I see more junior people taking their time to craft good commit messages and edit their commit history before pushing a PR.
- amelius 7y agoOk, but how much time have you "wasted" wondering what a piece of code was doing versus how much time would you have wasted writing good commit messages?
- deanmoriarty 7y agoIt has been an obvious net time gain for my projects. I do a lot of system programming, and often there are very complex reasons why a particular function is called in a non-obvious way, especially after tricky bugfixes. Having a somewhat detailed written record associated with the commit, that won't rot like an inline code comment would, has been very useful in my experience. And all my thoughts become instantly available to all the future code readers, who will most likely not be me.
- ses1984 7y ago>that won't rot like an inline code comment would It's a balance between rot and visibility, right? Unless you code with commit messages always visible, you have to go out of your way to look at them. If the interesting line changes even a little bit, you could hide the commit message explaining everything with one that explains a small change. If I could only have one, I would rather have comments visible in the code, and control they rot through change review, but of course finding a balance between both is the best.
- jgrahamc 7y agoTotally agree: https://blog.jgc.org/2013/07/write-good-commit-messages.html https://blog.jgc.org/2013/07/write-good-commit-messages.html
- deleted 7y ago[deleted]
- drej 7y agoA somewhat related discussion: https://news.ycombinator.com/item?id=21289827 https://news.ycombinator.com/item?id=21289827
- ASalazarMX 7y agoThis should carry over to version changes visible to the end user. "Stability and performance improvements" is the equivalent of "Fix bug" commits.
- mcslearner 7y ago> What's really sad is how often I find myself telling people this. It should be part of our culture... Somehow I don't think whining about this problem on some google group is going to help.
- practical_lem 7y agoIt isn't "some google group", it's the golang-dev mailing list.
- ivl 7y agoI like this. A large description of what's changed does a load of good. What kills me is when using pull requests, putting a complete description into the pull request... and then finding my commits not squashed after my PR is approved.
- MayeulC 7y agoWell, I usually prefer to write descriptive commit messages for individual commits before opening a pull request. If I feel it needs to be squashed, I will do it myself. It crushes my heart every time someone squashes my detailed, self-contained commits together. If there is more information in the PR than in the commit messages themselves, I will add that to the merge commit (see my other comment about merge commits).
- rb808 7y agoOne problem with commit messages is that they get overwritten. Often code is refactored multiple times so the original good quality commit messages are hard to find amongst upgrading/refactoring/moving/cleaning messages. Makes me wish for comments perhaps.
- ryanianian 7y agoGithub has the nice ability in its `git blame` column to show the previous commit for chunks in cases where somebody does a large-sweeping cleanup commit. I often wish git-blame had the option to show this as well.
- tuckerpo 7y agoI follow the format of the first line being a tag with a short one line description, followed by a new line, followed by further explaining of the what and why. Example: "Fix: router now asynchronous. Our previously implementation of a synchronous router was causing a 30% slowdown when tested with `wrk` (hyperlink to utility used to test)." with an additional link to a Jira ticket if the particular commit addresses one. Seems to float well with everyone I've worked with.
- jgalt212 7y agothis makes perfect sense and one should show care when constructing commit messages. That being said, if commit messages require too much care, then folks will start to make less commits as they become rather expensive to do. As with all things in life, when and how often to commit is a balance.
- iudqnolq 7y agoWhat do you think about committing with one-liners as you develop so that you can rollback, and then completely rewriting history before pushing?
- pavon 7y agoI absolutely agree that you should commit early and often, and that short one-liners for those commits are fine. It is when you go to merge that you need to have good messages and there are several ways you can do that, depending on your team's preferences: * You can rebase and groom your commits into meaningful sets of changes each with good descriptions (how Linux uses git). * You can squash all the commits, and always just have a single commit for each merge with a detailed description. * You can keep your full commit history as a record of how the work was actually done (Fossil encourages this philosophy), but always have a detailed merge commit that can stand-alone in describing all the work done in that merge.
- briandear 7y agohttps://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html https://tbaggery.com/2008/04/19/a-note-about-git-commit-mess... (2008)
- Pxtl 7y agoWe take a "PR all the things" approach using MS VSTS/DevOps. Every PR needs a work-item attached and a description, and those get cooked into the commit. We always squash, but squashing is dubious - if the any of the commits of the branch are still kicking around in other branches, it looks like they're ahead when they're not. Imho, this is a huge flaw in GIT - I don't want to squash, but I do want a clean history and I don't want to require that juniors and student developers waste their time curating their history. The fact that there's no way to mark commits as "this is unimportant crap leading up to the merge" in Git is annoying. VSTS logs the PR number and the relevant work-items in the message so for any given commit it's easy to get back to the PR where you can see the detailed discussion of the change, requirements, etc. But either way: reviewers enforce useful PR message, and we discard the intermediate commits. This works very well for clean history.
- SkyPuncher 7y agoIsn't that what rebasing is for? We rebase all of our feature branches for PR's.
- Pxtl 7y agoYeah, I just mean the PR process auto-squashes/rebases instead of merging, so that junior developers don't have to think about it. The problem is that if somebody leaves the branch up (delete-on-merge is obviously preferred), or derivative experimental branches, you can't tell if it's been merged or not because the commit hashes are different after the rebase. The branch list has become a scary basement.
- bendavis381 7y agoCheck out how phabricator handles this. It's essentially what you're asking for: https://secure.phabricator.com/phame/post/view/766/write_review_merge_publish_phabricator_review_workflow/ https://secure.phabricator.com/phame/post/view/766/write_rev... TL;DR: PRs are not based off a diff between master and a remote branch. Instead, phabricator sends up patches to code review and when approved, the Phabricator tooling handles the rebase locally. No remote branches and clean history.
- topaztee 7y agowrote a little post on just this https://www.topaztee.com/markdown/git-commit-message/ https://www.topaztee.com/markdown/git-commit-message/
- MayeulC 7y agoI also encourage everybody I know to write descriptive merge commits: "Merge feature X" instead of "Merge someone/master into master". It really helps when using git log --first-parent to avoid displaying intermediate messages. This really helps with the so-called "Aligator" [1] workflow: create a merge commit for every bugfix or feature, instead of just rebasing (you can still rebase if you want). That makes the history contain more metadata: a bit like a HN or e-mail thread can be collapsed to filter it out, first-parent helps filter the noise (implementation details) to display topics. I wish web UIs such as gitlab's allowed to collapse commit graphs according to this. [1]: https://euroquis.nl//bla%20bla/2019/08/09/git-alligator.html https://euroquis.nl//bla%20bla/2019/08/09/git-alligator.html (no mention of first-parent in that blog post, unfortunately.)
- ilikepi 7y ago> I also encourage everybody I know to write descriptive merge commits... Or alternatively, use a branch naming scheme that includes a little ticket metadata: "Merge branch 'feat_4714_default_tag_filter'"
- SkyPuncher 7y agoPersonally, I find little use to stashing a lot into a commit message. Nearly all work is managed outside of Git. Provide a short description, reference the work you're doing (a ticket, a PR, etc), and hold the discussion there. Rebase many, meaningless commits into a single one for integration purposes.
- RussianCow 7y agoI used to believe this as well, but then a company I worked for switched away from GitHub (we used GitHub Issues prior), and we lost all of that information. Commit messages are immutable, whereas anything else can easily disappear or become inaccessible.
- jedimastert 7y agoSomewhat important to note that he's talking about change list messages, not commit messages. Change lists are full sized chunky patches.
- sime2009 7y agoTrue. In git language that is PR level, not commit level.
- stefan_ 7y agoThen single best commit messages you are ever going to find in any given project are in the Linux kernel: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/ https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/lin...
- yodsanklai 7y agohttps://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6609fee8897ac475378388238456c84298bff802 https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/lin... Wow, that's a great commit message for one character change!
- kstenerud 7y agoThat's a lovely description of why the fix was needed, but I just took a look at the go source, and NOWHERE IN THE SOURCE CODE IS THIS FACT DOCUMENTED. Signals are tricky, temperamental beasts; exactly the sort of thing you'd want to have comments about in your code. The way things stand, nobody would ever know why the signals are set up the way they are unless they trawled through 40,000 commits. Yes, 40,000: git log --oneline | wc -l. I'm all for giving a helpful description of why a commit was needed, but this is FAR too detailed for a commit message, and belongs in the CODE, which people will actually be reading while they debug an issue. As for his other examples of bad commits, "Moved A to B" is not necessarily bad, unless there was a reason to do it other than to just rearrange where things lie. Same goes for "Add convenience functions" or "Code cleanup". If you're not fixing a bug or adding new functionality, you're just doing janitor work. And that doesn't require detailed descriptions. Commit messages are to let people know in a concise way why you made the commit. Comments are to let readers know why you've done things in an unexpected way, or to describe things they're not expected to be familiar with, or tricky gotchas.
- fdr 7y agoDo not agree. Commit messages can be more expansive than comments, because comments bitrot more readily: they are not rooted in a time and change to the code. I use commits for the largest bulk of explanation, particularly "why," which can be rather detailed and not of much interest to someone skimming the code rapidly. Comments I use for pointers that are unlikely to bitrot and/or important to prevent error. Except for the most timeless design elements in a project, I do not commit long documentation.
- stinos 7y agoI do follow your reasoning, bad comments are worse then no comments, but in this particular case it really feels to me like the meat of the commit body should also be a comment. Just so that when someone looks at the code and wonders, hmm, why is this flag here, he/she can figure it out without having to do a blame/log on the file. (Don't get me wrong, great tool, but has it's own problems). Judging whether or not comments are permitted on the bitrot-ability of a piece of code seems hard to begin with, and even then a bit arbitrary. In this case, the whole comment would be just for that flag. So if someone really just gets rid of the flag but not the comment, especially on code like this, and that also passes code review then I think there are more pressing problems then bitrot.
- clktmr 7y agoI recently started to write git commit messages as if I was writing an email to my future self. It enables me to write the commit message without thinking too much about what's important. I also tend to write more since, because an email with just one paragraph is pretty rude.
- Vysero 7y agoIt seems like what was said by Austin Clements was more of a bug report than it was a description of a solution for a bug. Which is the job of reporter and not the person fixing the problem. In my mind, a commit message should not be the be all end all for what can be said about a particular bug fix. If someone needs to know the details then they can look at the diff and if the person who made the change feels it was complex enough then they should add a comment on the bug report about what was changed and how it was changed prior to marking the bug as *fixed.
- hinkley 7y agoI can't help but feel a sense of cognitive dissonance in the fact that we put such a large degree of importance on a write-once piece of data. If getting the comments "right" on commits is really that important, then you should be able to revise them based on new information. Are they important, or are they only important because you have to get them right the first time so you have to treat them that way? (That's a bit of a rhetorical question. I get quite a lot out of walking the commit histories on code, but often it's the shape of the commit that contains the information, not the comment)
- MaxBarraclough 7y agoPike's summary seems pretty spot on: What, why, and background. Right on. I really don't understand the reply in that thread, "what" doesn't matter because it's in in the code. Of course it matters. The whole point is that you get your bearings from the comment text, and you only need to look at the code to see the details of the 'how'.
- deleted 7y ago[deleted]
- deleted 7y ago[deleted]