Rob Pike on good commit messages (2014)(groups.google.com)
groups.google.com
Rob Pike on good commit messages (2014)
https://groups.google.com/forum/#!topic/golang-dev/6M4dmZWpFaI
97 comments
Do 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.
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.
I 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.
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.
> NOWHERE IN THE SOURCE CODE IS THIS FACT DOCUMENTED.
The referenced commit (https://github.com/golang/go/commit/675eb72c285cd0dd44a5f280...) has /* SIGSETXID; see issue 3871, 9400 /, / SIGCANCEL; see issue 6997 */
> 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
> 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
At Google (the original audience of the "rant") tools like Moma and Code Search make it much easier to find the relationship between commit messages and code, so it makes more sense to avoid duplicating information in comments. Finding the commit message for a line of code literally takes one press of a button.
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.
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.
Having worked at Google, agree 100%. I think people at Google expect people to look at the commit message more than at other places because it's so easy.
I agree that commit messages aren't a good place for detailed documentation.
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.
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.
> "Moved A to B" is not necessarily bad
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.
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.
Git tools may be routed in email, but they most certainly aren't used that way by most people. Why would I write a commit message like I'm writing an email if both the commit message and email are the wrong tools for the job? We have issues are PRs for code discussion within the context of code. This priceless information should live with the repo. Not in some isolated silo like email which is not accessible to people who aren't on the chain, or commit messages that are difficult and time consuming to traverse in any but the simplest and smallest repos.
Fossil[1] integrates a bug tracker & wiki into the repo proper, and I agree that it would be good for these to just stay pinned together in general. I'm kinda sad that git has basically "won" the SCM wars in the FOSS world; a decade ago there was a lot of interesting experimentation happening and I feel like having a winner has caused that to stagnate.
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.
[1]: https://fossil-scm.org
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.
[1]: https://fossil-scm.org
> and NOWHERE IN THE SOURCE CODE IS THIS FACT DOCUMENTED.
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...
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...
After 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.
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.
I've been a Software Engineer for 15 years now... 10 years in the industry as a systems engineer before that.
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.
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.
My editor (Sublime + GitGutter plugin) is set up so whenever I click in a certain line of code, it displays a hint out to the right with the summary line of the most recent git blame on that line. It's pretty fast...maybe 1/2 second delay. I've found this really, really useful. As you say, commit logs can hide unless you're looking for them, but very often, the most recent blame (and even just the summary line of that blame) gives me really useful information.
It doesn't need to be an either/or affair. Often times, I'll add a comment to a particularly tricky piece of code, but then also comment in the commit message (and then again in the PR in Github for the code reviewer)
Often times, even a copy-paste is better than nothing. Better to over-communicate
Often times, even a copy-paste is better than nothing. Better to over-communicate
Jetbrains can be set to show you the last commit message that modified a chunk of code. I assume there's a way to do this in vim/emacs for the sufficiently motivated as well.
This is basic functionality via `git annotate` (which goes back, at least, to `cvs annotate`).
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.
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.
Awesome. Was missing that in emacs. Magit continues to surprise and delight me.
I think part of the issue is that for someone who's been writing English for 15 years, knocking out 2 paragraphs of cogent explanatory prose is as easy as ABC.
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.
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.
> But for someone who is relatively new to the English language
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.
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.
Fundamentally, I think you are correct, however my English-second-language friends/colleagues tend to write better English when it's in a structured fashion - like a commit message or in documentation - than they do colloquially. That is how they learned it, after all, while native speakers learn it from osmosis. It's rare to see the ESL speakers use the wrong there/their/they're like native speakers do.
My first full time job was at a university, and we had a lot of student interns, most of them not native speakers (lots of chinese students especially). What I've found is that it wasn't worth the trouble to correct every grammar mistake; if a commit message was understandable and had useful content, I'd usually merge it without bothering to correct botched plurals, wrong prepositions, and other mistakes that the students made regularly, as long as it didn't hurt understandability. I found that getting them to write good content wasn't a problem once we made it clear that this was expected (and it wasn't really easier with the native speakers).
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.
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 can somewhat relate to that, since I am not a native English speaker and it took a while for me to become proficient with the language, but I can say that in the teams I worked in it was mostly a matter of sloppy culture.
every single commit I push has an entire paragraph or two clearly describing the changes, and the motivation behind them
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 :)
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 :)
What in your opinion is the benefit/difference of this, versus commenting nonobvious code? I tend to prefer the latter. Each time I work on someone else's code, I'm elated if they write longform comments about what they're doing or why. I think tracking down commit messages would be more cumbersome.
Good comments are important too. But comments describe the code as it is at this point in time ("why is the code doing X?"); commit messages describe the change ("why are we making the code do X rather than Y?"). Sometimes the same information should go in both places; but the focus and emphasis when you're writing the description up varies.
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.
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.
Both good points to consider - thank you.
To add to that, probably at length:
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.
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.
I tend to put comments around VERY tricky pieces of code, but the rest belongs to the commit log.
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).
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).
In my experience I don't see them as mutually exclusive. Good comments in code should still be in place. In my opinion the git history should not explain what the code does necessarily but instead the nature of the changes that were made. In my experience projects are significantly easier to manage if commits are squashed into larger commits that have (1) a clear title, (2) clear summary of what was done, and (3) some link to the tickets in JIRA with the full details. In the short term it may not make a big difference but once you have years of history in a single project as well as many hotfixes or releases the better organization of git makes the project easier to manage.
When a straightforward piece of code is replaced by another, it can still be useful to know why.
Being able to seamlessly browse the history of a given function is one of the things a fully integrated development environment should support.
Being able to seamlessly browse the history of a given function is one of the things a fully integrated development environment should support.
Assuming you're doing Git based flow branching off of Develop or Master to do changes. Before merging your changes back in do you rebase and cleanup the messages or squash? I feel like every company I work for just does merges everywhere and history is 99% unreadable at this point without filtering out certain things on the log and even then.
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".
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".
We disabled merge commits on all of our repositories on Github, and rebase is used seldom - meaning that when you do merge you are presented with an edit box for the squashed commit message. This incentivises one to actually write good prose inside that box. We are not perfect but getting there.
That's interesting. Working in an organisation that switched from Gerrit to Github, I saw a decline in commit message quality. Some people will just leave whatever is filled into the textbox by GitHub. In addition, there is no way for others to review the commit message this way.
Ok, 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?
>> how much time would you have wasted writing good commit messages?
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.
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.
It's a balance, but I've definitely seen that a good commit title can massively help newcomers, yourself when you can't remember why something is the way it is and can be a boon when debugging.
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.
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.
This is what I do, means I don’t have to worry about describing a “fix linting errors” commit. Just on squash merge ensure the commit message captures what’s required (and on gh the pr is automatically included and I typically put any linked issues in the additional info area)
I can see what the code is doing. I can't necessarily see why it's doing that and whether it's a bug or a feature. Good messages make ""blame"" extremely useful.
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.
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 can see what the code is doing.
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.
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.
It's not wasted, sloppy commit messages are correlated with sloppy code.
Thinking through describing the change in prose has an immediate positive impact on code quality.
Thinking through describing the change in prose has an immediate positive impact on code quality.
It 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.
>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.
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.
> Unless you code with commit messages always visible
I do this when tracking down bugs or trying to figure out the design of a particular component.
I do this when tracking down bugs or trying to figure out the design of a particular component.
For me it's definitely in the first category, and I take my time writing good commit messages.
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.
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.
> 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.
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.
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.
We tag each commit with the (JIRA) ticket ID. The ticket has the broader business context, and the commit message is generally shorter. There's still some gap between the one-line commit message and the ticket ID, but it hasn't really caused problems in our org to my knowledge.
I also think it's more important to think of a commit as a performance piece. The words matter but the shape is more durable.
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.
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.
We have a precommit hook that insists on a ticket being in the commit message, which helps.
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.
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.
At about the same time, Chris Beams wrote this, which I have loved ever since:
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
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*> 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.
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.
> 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.
Usually that kind of stuff goes in the body of the commit message.
But 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.
but 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.
I 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.
Okay, well how do we enure that we get this without some guidance? It's very frustrating.
Do the team already have a consensus? Do they believe there’s a problem?
Interestingly, in the 3rd post in the submission, Pike specifically avoids mandating a style guide.
> 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.
> 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 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.
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.
> 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.
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.
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.
Yeah, for me the approach is that the PR describe the what that was done at high-level, and the requirement describes the why.
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.
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.
> I don't want to require that juniors and student developers waste their time curating their 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.
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.
Isn't that what rebasing is for?
We rebase all of our feature branches for PR's.
We rebase all of our feature branches for PR's.
Yeah, 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.
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.
Check out how phabricator handles this. It's essentially what you're asking for: 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.
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.
That's an interesting fix although I'm unsure about the sense in discouraging pushing changes.
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.
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 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
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.)
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.)
> 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'"
Or alternatively, use a branch naming scheme that includes a little ticket metadata: "Merge branch 'feat_4714_default_tag_filter'"
Then 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/lin...
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/lin...
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/lin...
Wow, that's a great commit message for one character change!
Wow, that's a great commit message for one character change!
Somewhat important to note that he's talking about change list messages, not commit messages. Change lists are full sized chunky patches.
True. In git language that is PR level, not commit level.
I 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)
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)
Pike'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'.
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'.
One 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.
Github 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.
One 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.
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.
I 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.
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.
It 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.
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.
This should carry over to version changes visible to the end user. "Stability and performance improvements" is the equivalent of "Fix bug" commits.
I 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.
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.
this 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.
What do you think about committing with one-liners as you develop so that you can rollback, and then completely rewriting history before pushing?
I 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.
* 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.
I 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.
Personally, 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.
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.
I 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.
A somewhat related discussion: https://news.ycombinator.com/item?id=21289827
wrote a little post on just this
https://www.topaztee.com/markdown/git-commit-message/
[deleted]
I 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.
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.
Well, 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).
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).
> 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.
Somehow I don't think whining about this problem on some google group is going to help.
It isn't "some google group", it's the golang-dev mailing list.
[deleted]
[deleted]
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.