3 ms·

> 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 th

> 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.

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.