Rob Pike on good commit messages (2014)
groups.google.com
groups.google.com
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.
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.
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.
There is always a reason why things are 1) done and 2) done in this specific way and not another. It is also relevant if this is part of a larger change and it there are external dependencies on this commit. These things must be clear from the commit message.
> you're just doing janitor work
That makes it sound like janitorial work does not require complete commit messages which is why we sometimes see useless messages. There is always a reason why something was done and this can be documented.
A bug can be documented why it happened, why it was fixed in this specific way and not some other more obvious way, and what steps has been taken not avoid it from resurfacing. The same way a clean up can describe why others paths were not taken, why the new way is considered more clean (fewer lines, lesser complexity?) and how we know it does not entail functional changes.
The git tool is rooted in email, and commit messages look a lot like it. Pretend you are describing your patch over email to someone and it will be almost trivial to write.
Re: Email being an inaccessible silo: communities that use git-format-patch/git-am usually use mailing lists, which are archived, and subject lines have [PATCH] in them so searching needn't be difficult.
Another reason is that if you had a mistaken understanding when you made the commit and wrote up that understanding in the commit message, you're creating a greater risk of misleading future readers than if you'd put the writeup in a file which can itself be version-controlled.
I think the right place isn't necessarily code comments; separate internals documentation (in the same repo as the code) can be good too. Then the comment near the code can just be a cross-reference.
If it's really tedious to find relevant commit logs then I think the right answer is to improve the search process rather than compensate with more comments.
> 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.
git blame exists. 675eb72c285 src/runtime/os1_linux.go (Austin Clements 2014-12-19 16:16:17 -0500 417) var sa sigactiont
Nowadays that I use VS code with GitLens, each line of code automatically shows me its latest commit message, just hover that info and I have the date, author and a link to the changes which usually give some useful context to understand why and how some particular line was edited. Very useful!
Too many times have I seen well documented code only to later realize that the code was edited with no updates to the comments, now useless and a source of time waste...
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.
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.
I do this when tracking down bugs or trying to figure out the design of a particular component.
It was a constant source of irritation at a previous job where the repository had been imported from a different version control system without history. So half the time "svn blame" would just tell you it was in commit #1 ten years ago, which wasn't nearly enough.
I don't think the reader should have to look at the code to get the summary of what the code does and how it does it.
I agree that the "why" should be the focus.
or time wasted writing unit tests, or pull requests, or code reviews?
(1) It's about effective communication. Why do developers appear to care so much about the way we write our code but not the artifacts that form it's ecosystem (and have a much wider audience)?
(2) If you're going to write a commit message, why not take the incrementally very tiny extra bit of time to do it right?
If you don't really care about the quality of your commit messages I'd suggest stop requiring them and allow blank commits. IMO this is preferable over the false impression that a garabage commit history provides any value.
A nice balance I've found is to not care while developing, but then squash all commits into a single one when merging a pull request and give that a good message.
Thinking through describing the change in prose has an immediate positive impact on code quality.
It also serves as a good filter for commit content. If you can't write a good, comprehensive commit message for it, it probably shouldn't be its own commit.
But for someone who is relatively new to the English language, this can be the source of much anguish and frustration. Doubly so for something like a commit message that is immutable by design. The idea of a typo, a bad idiom, or even worse, unintentionally offensive phrasing, being surfaced for all to see 10 years from now can be a frightening prospect.
I think this whole discussion goes to show the nature of institutionalized privilege, and how seemingly innocent requirements can have a disparate impact on marginalized groups.
We decided to structure our front-end application using Atomic Design which classifies UI Components in a hierarchy of Atoms, Molecules, Organisms, Ecosystems, and Pages. Pretty universally the definitions of these terms did not translate to many of our remote developers in other countries not only because of the language but likely they didn't have the same Biology education either.
This and other similar revelations have led me to always put myself in the shoes of anyone reading my code, comments, commit messages, and force myself to use simpler language whenever possible.
Communication in software development is a huge deal. While language barriers are a thing, I don't think you can or should compromise on documentation, and when it comes to commit messages I don't think this is the leading reason why folks don't write good ones.
I'm told by friends who were at RedHat when they decided to make the source repositories public (as opposed to just throwing release tarballs over the wall), and commit messages went way way up. This tells me: people knew better, they just didn't care if they weren't being held to account.
I feel code comments tend to rot because they are not as immutably bound to the code they describe as a commit message is, and for the most part my commit messages are too verbose to end up in code comments, it would create too much clutter.
I have a good integration with my editor that shows inline blame annotations for the current line (and full message is a keystroke away).
A good commit message is also really useful for code reviewers: it sets the stage and contextualises all the code they're about to read.
Some information is a pre-requisite for understanding a specific block of code. This information is a great fit for comments, because everyone who's looking at the code will need to know it, and having it in a comment ensures that it's discoverable.
Other information is a pre-requisite for understanding the engineering or business decisions that motivated a design decision, but isn't necessary for understanding how the code works or how to interact with it. That makes it less appropriate for comments, because the comment would just be clutter (light pleasure reading at best) under the most typical use case. People who are engaging in those use cases should know how to use git blame to find what they need.
For still others, the unit to which the information applies is not a block of code; it's a set of changes. In those cases, comments are a terrible choice for conveying that information. Maybe a commit message is better. If it's not, you probably need to be looking to a project wiki or some separate documentation files, so that the information can be written and maintained in one place. You can still use comments, but they should merely cross-reference the centralized documentation. Today's copy/paste is tomorrow's outdated and misleading misinformation.
Being able to seamlessly browse the history of a given function is one of the things a fully integrated development environment should support.
I sort of agree but what I try to do is put more comments and documentation in the source code directly.
The problem is that commit messages are usually hidden only when there's a problem.
Comments are there for the life of the code.
The problem with comments though is that they could go stale and something could be refactored.
I try to put them directly on methods/functions more and not on specific lines of code.
If I have to document a line of code it probably is a sign that it should be a function/method itself as it's a bit too funky to just stand alone.
Commit messages are GREAT for tracking down when something is broken though.
Would be nice to have your IDE take your code comments and make commit messages for new methods and the documentation for that method/class.
It is also built into Emacs as `vc-annotate` (default `C-x v g`), though `magit-blame-addition` is a better option for projects using git.
Often times, even a copy-paste is better than nothing. Better to over-communicate
Any tips on how you're able to keep things clean for the entire team ? What's your usual workflow? I've read through Martin Flower and a few other blogs on this topic yet have never understood how to avoid "add x", "fix foo".
I used to do this, but prefer the list form. I like to note what has been added or removed, if you're searching through the commit history it's all useful.
Similar here, but: looking back at a couple of years ago I obviously went through a phase where I was just writing complete paragraphs for the sake of making sure the commit looks meaty. So now I treat messages the same as code: code not written is good. If a one-iner is enough to explain why something was done, and the changes in the code speak for themselves, than that's it.
even if it's a one-line diff.
By now you probably figured the size of the diff doesn't seem to have much of a relationship with the amount of commit message needed :)
Except some idiot migrated our bug database to a new server and re-numbered the tickets in the process, so only the last two years are intelligible.
You can comment the commit all you want, but if it contains three separate and unrelated activities then the commit will still be 'muddy', whereas 3 commits with bad comments but one activity per is far more legible.
Remember, nobody is reading your commit messages, your tests, or (probably) your documentation unless something is already wrong. Then they're already stressed out and trying to grovel through your information. They're already biased toward being grumpy. Don't add onto that. I'm only reading your commits trying to figure out if a bug is a feature poorly realized or not, so I don't break something else.
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*Okay, well how do we enure that we get this without some guidance? It's very frustrating.
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.
Usually that kind of stuff goes in the body of the commit message.
> I would rather not turn this into rules and style guides, just a widespread understanding that a changelist description is worth taking some time to write.
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.
We rebase all of our feature branches for PR's.
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.
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.
It seems like the chief benefit they tout - clean history - is already handled by the auto-squash-and-delete approach. The real problem is the difficulty in cleanup of secondary or undeleted branches.
It isnt' really what I was asking for, though. What I want is a "soft-squash" where the intermediate commits aren't deleted, rather they're just hidden from history. The ability to draw a dotted line around a region of the DAG and say "treat this blob like it was one commit for all intents and purposes unless I say otherwise.
I've worked on a couple project that took the approach of documenting the details of what was done in the bug tracker (or PR/MR), etc, and then just putting the bug# and a one-line description in the commit history.
In every one of these projects, we ended up later moving to a different bug tracking system. In some cases the bugs were transferred, but the numbers were not, in others nothing was transferred. Suddenly all those bug numbers in the commit history were completely meaningless and useless.
I'm now a strong advocate that the detailed documentation should be in the commits messages themselves. One of my biggest complaints with gitlab is that you spend all this time writing a good MR description, and then it has no mechanism for including that description in the actual merge commit.
That said, MS is good at long-term support if nothing else. Our TFS/VSTS/DevOps instance is older than dirt so I don't have to worry about losing history.
Uhm, one of just the most important skills for junior developers to learn is to tackle a larger change through a succession of individually logical, atomic changes.
git log --first-parent
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 (no mention of first-parent in that blog post, unfortunately.)
Or alternatively, use a branch naming scheme that includes a little ticket metadata: "Merge branch 'feat_4714_default_tag_filter'"
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/lin...
Wow, that's a great commit message for one character change!
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)
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'.
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.
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.
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.
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.
* 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.
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.
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.
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).
Somehow I don't think whining about this problem on some google group is going to help.