5 ms·

I look at commit messages every day. I actually don't love a message that just describes the code change (I can figure that out on my own if I look at it long e

I look at commit messages every day. I actually don't love a message that just describes the code change (I can figure that out on my own if I look at it long enough). That's fine for bigger or more misleading changes, but when I'm looking at blame/history I'm usually very aware of what the code is doing already. My question at those times is less about "what" and more about "why". Why was this change made? Sometimes that can be found by looking at accompanying tests (if the original author was so gracious), but if it's not in the commit (or only obtusely so, as many developers I know tend to commit in what I'll call "stream-of-consciousness" style) then I've got to go outside the codebase to research by interviewing people (assuming they're still around and remember) or trawling through our project management system to try and line up released features/fixes with the change. It quickly becomes an inexact science. That said, in my experience, the larger an organization, the more commit messages get looked at. On my side projects where only 1-3 people are involved, commit messages only really show up on the PRs as things are going in. Sometimes we complete our own PRs to keep up the velocity since we're all working async part-time. Rarely if ever do we look back to figure out why we did something because the project is so small.

BoxFour 3y ago
> My question at those times is less about "what" and more about "why". Why was this change made? Isn't the essence of the commit message body to delve into the 'why'? The ability to swiftly glean the 'what' from the commit subject line aids in identifying the appropriate commit for review. The commit message body can then have a detailed explanation of the reasons behind the change.
- jbverschoor 3y ago
  The diff is the what. Putting what in the message is fluff to make gatekeepers happy
  - masklinn 3y ago
    Disagree. Putting "what" in the message can be fluff, in the same way repeating the code in a comment is fluff. But the broad strokes or considerations of the what may not be obvious from the diff. Expressing what the commit intends as prose can help with understanding, with discarding irrelevant change data, with post-mortem analysis, ...
    - vitus 3y ago
      It is also useful for conveying intent. If the commit message describes one thing, and the code does the opposite (or doesn't do it at all), that's something the "gatekeeper" can and should call out. I've certainly sent out changes for review where I forgot to include a crucial file (or subdirectory).
      - withinboredom 3y ago
        About to say this as well, but would like to add. When I have a bug dealing with Foo in Bar, seeing a commit message like “Updated Foo to support Baz\nSee issue #123 for additional context. Also discussed in chat (link).” When looking for a recent bug, I’m going to review that commit immediately without having to do any bisecting/blaming. Easy peasy. Mostly though, I tend to see just “support baz” as a commit message, which drives me bonkers.
        hmeh 3y ago
        I'll offer a perhaps controversial perspective. What if only people that had context did reviewing? As in, what if when someone needed a review, it was their job to ensure that the reviewer had the context necessary. That is, it was "push" rather than "pull". What if a team's process was set up in such a way that separate phase-gate reviews were not typically necessary. That is, the team tended to work in pairs where there was constant reviewing going on, and the team was practiced at pulling people in when necessary to review specific parts, e.g., things they had never done before or needed additional guidance on. Do we all remember waterfall development? Where PMs would write specs, then they would throw those specs over the wall to devs, who would then throw code over the wall to testers? Does anyone feel like, perhaps, we've done it again with pull requests and we've started favoring asynchronous hand-offs, rather than one-piece-flow?
  - BoxFour 3y ago
    In bigger teams, scanning every diff instead of just the commit subject lines is significantly more time-consuming (a frequent activity in my experience). I often decline proposed commits that lack concise, informative subject lines, and I hope other larger teams also do for efficiency and clarity. If that's considered gatekeeping, then I'm content to be one.
  - g-b-r 3y ago
    Understanding a diff usually requires knowing and remembering the codebase well, having a very clear code, the diff being reasonably short and atomic and time. Commit messages, if good, are extremely helpful to check what changed e.g. in an open-source application you're using. Of course the commit messages can lie, so you should also check the diffs, but it's much easier to confirm what a diff does than deducing it. Not to mention that for many changes you can only make a guess at what the intention was; if you have a message saying what was meant to be done, you can check that it really was done correctly.
  - ordu 3y ago
    There are different levels of "what". 1. I'm moving value from this address to that address through %rax register. 2. I'm copying this value from that place to that. 3. I'm making sure that this values are equal. 4. An invariant was broken and I need to restore it. If you look at it, each next like is an answer to "why" question applied to a previous line. Answers to "why" questions becomes new "what" and then "why" applied to them again. The point I want to make is it is easier to move back (in descending order) through this than forward. When you know (4) you can easily figure out what is needed to be done. But if you have (1) you need to infer next points one by one. One who just wrote the patch is probably unaware of differences, because he knows all the answers and can easily move from one to another. He/she do not feel difficulties. But the person who reads patch starts with (1) and needs to climb the ladder in opposite direction than the person who wrote the patch.
masklinn 3y ago
I agree with your first paragraph, but I don't think it's super relevant to the article: the why of a commit usually takes some time to explain, and the subject line is nowhere near sufficient, so that usually goes into the first paragraphs. For non-trivial commits I try to follow the "why, what, how" template: first explain the issue / concern / background for the change, then clarify the change if the diff is not super obvious or is a bit large, and finally explain the implementation changes if there were options. In my experience the former and latter are the most important when blame-diving, I want to know the context / driving force / use case for a change, but I also want to know the scope of consideration, because e.g. sometimes I'm thinking of a fix which was considered and rejected for still-valid reasons, while at other times it was just considered unnecessary back then (YAGNI) or the blockers have since been removed. Either way the subject needs to be snappy, so that it can quickly be selected for consideration or not e.g. if I'm blame diving and I see a commit which just does indenting / formatting, I probably can skip over it without looking at the actual change.
- twodave 3y ago
  I mostly agree, though if I could only have “what” or “why” I would choose “why” every time. To me this is similar to describing the problem in product design. I wish the “why” was more often concise enough to be the prominent feature in a commit, because commits are difficult/annoying to amend (I’d be surprised if more than a couple people where I work knew how) and if something is going to be left out, it is the “why”.
  - masklinn 3y ago
    Can't disagree with that. I think the order of importance is why > how > what, because the first two are the least likely / possible to infer from the commit contents. So an explicit record is most crucial.
o11c 3y ago
> when I'm looking at blame/history Those are two very different things when it comes to commit messages. For history, the linked argument is arguing that subject-first is better. Personally I'd prefer `[component] verb details` though, which should fix the skippability issues mentioned elsewhere. And of course you can enable diffstat to get a bit of an idea of the structure. For blame, you have the full file context in front of you. Some blame frontends only show author, but assuming you have the commit message, what you really want out of it is a rough idea of the historical significance of that line. So definitely verbs first.
Hamuko 3y ago
>I actually don't love a message that just describes the code change (I can figure that out on my own if I look at it long enough). Isn't the point of the commit message title precisely that you don't have to look at it long enough?

BoxFour 3y ago

> My question at those times is less about "what" and more about "why". Why was this change made? Isn't the essence of the commit message body to delve into the 'why'? The ability to swiftly glean the 'what' from the commit subject line aids in identifying the appropriate commit for review. The commit message body can then have a detailed explanation of the reasons behind the change.

jbverschoor 3y ago
The diff is the what. Putting what in the message is fluff to make gatekeepers happy
- masklinn 3y ago
  Disagree. Putting "what" in the message can be fluff, in the same way repeating the code in a comment is fluff. But the broad strokes or considerations of the what may not be obvious from the diff. Expressing what the commit intends as prose can help with understanding, with discarding irrelevant change data, with post-mortem analysis, ...
  - vitus 3y ago
    It is also useful for conveying intent. If the commit message describes one thing, and the code does the opposite (or doesn't do it at all), that's something the "gatekeeper" can and should call out. I've certainly sent out changes for review where I forgot to include a crucial file (or subdirectory).
    - withinboredom 3y ago
      About to say this as well, but would like to add. When I have a bug dealing with Foo in Bar, seeing a commit message like “Updated Foo to support Baz\nSee issue #123 for additional context. Also discussed in chat (link).” When looking for a recent bug, I’m going to review that commit immediately without having to do any bisecting/blaming. Easy peasy. Mostly though, I tend to see just “support baz” as a commit message, which drives me bonkers.
      - hmeh 3y ago
        I'll offer a perhaps controversial perspective. What if only people that had context did reviewing? As in, what if when someone needed a review, it was their job to ensure that the reviewer had the context necessary. That is, it was "push" rather than "pull". What if a team's process was set up in such a way that separate phase-gate reviews were not typically necessary. That is, the team tended to work in pairs where there was constant reviewing going on, and the team was practiced at pulling people in when necessary to review specific parts, e.g., things they had never done before or needed additional guidance on. Do we all remember waterfall development? Where PMs would write specs, then they would throw those specs over the wall to devs, who would then throw code over the wall to testers? Does anyone feel like, perhaps, we've done it again with pull requests and we've started favoring asynchronous hand-offs, rather than one-piece-flow?
- BoxFour 3y ago
  In bigger teams, scanning every diff instead of just the commit subject lines is significantly more time-consuming (a frequent activity in my experience). I often decline proposed commits that lack concise, informative subject lines, and I hope other larger teams also do for efficiency and clarity. If that's considered gatekeeping, then I'm content to be one.
- g-b-r 3y ago
  Understanding a diff usually requires knowing and remembering the codebase well, having a very clear code, the diff being reasonably short and atomic and time. Commit messages, if good, are extremely helpful to check what changed e.g. in an open-source application you're using. Of course the commit messages can lie, so you should also check the diffs, but it's much easier to confirm what a diff does than deducing it. Not to mention that for many changes you can only make a guess at what the intention was; if you have a message saying what was meant to be done, you can check that it really was done correctly.
- ordu 3y ago
  There are different levels of "what". 1. I'm moving value from this address to that address through %rax register. 2. I'm copying this value from that place to that. 3. I'm making sure that this values are equal. 4. An invariant was broken and I need to restore it. If you look at it, each next like is an answer to "why" question applied to a previous line. Answers to "why" questions becomes new "what" and then "why" applied to them again. The point I want to make is it is easier to move back (in descending order) through this than forward. When you know (4) you can easily figure out what is needed to be done. But if you have (1) you need to infer next points one by one. One who just wrote the patch is probably unaware of differences, because he knows all the answers and can easily move from one to another. He/she do not feel difficulties. But the person who reads patch starts with (1) and needs to climb the ladder in opposite direction than the person who wrote the patch.

masklinn 3y ago

I agree with your first paragraph, but I don't think it's super relevant to the article: the why of a commit usually takes some time to explain, and the subject line is nowhere near sufficient, so that usually goes into the first paragraphs. For non-trivial commits I try to follow the "why, what, how" template: first explain the issue / concern / background for the change, then clarify the change if the diff is not super obvious or is a bit large, and finally explain the implementation changes if there were options. In my experience the former and latter are the most important when blame-diving, I want to know the context / driving force / use case for a change, but I also want to know the scope of consideration, because e.g. sometimes I'm thinking of a fix which was considered and rejected for still-valid reasons, while at other times it was just considered unnecessary back then (YAGNI) or the blockers have since been removed. Either way the subject needs to be snappy, so that it can quickly be selected for consideration or not e.g. if I'm blame diving and I see a commit which just does indenting / formatting, I probably can skip over it without looking at the actual change.

twodave 3y ago
I mostly agree, though if I could only have “what” or “why” I would choose “why” every time. To me this is similar to describing the problem in product design. I wish the “why” was more often concise enough to be the prominent feature in a commit, because commits are difficult/annoying to amend (I’d be surprised if more than a couple people where I work knew how) and if something is going to be left out, it is the “why”.
- masklinn 3y ago
  Can't disagree with that. I think the order of importance is why > how > what, because the first two are the least likely / possible to infer from the commit contents. So an explicit record is most crucial.

o11c 3y ago

> when I'm looking at blame/history Those are two very different things when it comes to commit messages. For history, the linked argument is arguing that subject-first is better. Personally I'd prefer `[component] verb details` though, which should fix the skippability issues mentioned elsewhere. And of course you can enable diffstat to get a bit of an idea of the structure. For blame, you have the full file context in front of you. Some blame frontends only show author, but assuming you have the commit message, what you really want out of it is a rough idea of the historical significance of that line. So definitely verbs first.

Hamuko 3y ago

>I actually don't love a message that just describes the code change (I can figure that out on my own if I look at it long enough). Isn't the point of the commit message title precisely that you don't have to look at it long enough?