8 ms·

> Never say what you ware doing (it's obvious this commit is updating build scripts) DEFINITELY say what you are doing. And then say why. This blog post is a

> Never say what you ware doing (it's obvious this commit is updating build scripts) DEFINITELY say what you are doing. And then say why. This blog post is a good description of the most prevalent convention in how to write good and useful messages (e.g. this is what the Linux Kernel does and given the history of git there's some flow down): https://cbea.ms/git-commit/ https://cbea.ms/git-commit/ The key thing that means you should start with what is having a short one-line summary of what you're doing so you can look at a simple log and see which commit you're interested in.

pferde 5y ago
Also, if your commit message has parts that say "also, this", "also, that", don't be lazy and break those alsos into separate commits!
- jacobmischka 5y ago
  Not necessarily. It sounds like all of the things mentioned in the commit message are interconnected (updating build scripts and updating gitignore to remove the new build directories). It would be pointless to break these into separate commits, you can't revert just one without reverting the others. The commit message is lazy, but breaking these into separate commits because of some "also" rule of thumb is just blindly following a rule for the sake of it.
  - capableweb 5y ago
    > breaking these into separate commits because of some "also" rule of thumb is just blindly following a rule for the sake of it Agreed, cargo-culting is always bad! But, without knowing how OBS development team works, sometimes it does make sense to split commits like this, mainly if you have a larger repository where different people are responsible for different areas in the codebase. For example, build-scripts sometimes gets managed by other people than the ones writing some core algorithms, who are also different from the people writing public APIs to be consumed by users. In those cases, it makes reviews a lot easier when you can split out to review different commits to different people, instead of having everyone navigate the same commit. But again, not familiar with the internals nor the development cycle that OBS keeps, so maybe this doesn't make sense in this case.
    - threeseed 5y ago
      As others have mentioned many times here. This commit is based off a branch that was adding support for Apple Silicon. So the entire PR is not going to look clean until that branch is merged first.
      - capableweb 5y ago
        Yes, makes a lot of sense. Sorry if I gave the impression that I think somehow this PR is wrong or similar. I was just replying to the fact that sometimes splitting up commits makes sense, outside of this PR.
    - PostThisTooFast 5y ago
  - rlpb 5y ago
    My exception to the "also" rule of thumb is that if the split changes would cover separate areas of the code such that each piece could be easily picked out by "git add -p", then splitting them out isn't required as long as they are all associated to one logical larger story. > ...you can't revert just one without reverting the others That's often true in a split out patchset anyway, so I don't think that on its own it stands as a justification for doing things one way or the other.
- Cthulhu_ 5y ago
  Dubious. I prefer that style myself, but there's a lot of value in squashing multiple commits around a feature into a single, "this commit implements this feature" commit. Is it valuable to retain the entire development history of a feature, or just the feature itself? What if during the development, it was rewritten a dozen times? What if there's a dozen commits where the author meandered and only in the last three commits got to the point? Ruthless interactive rebase is one way to fix that last point, but if you take the final result and squash that into a single changeset, there's a point to be made about that as well.
  - pferde 5y ago
    Agreed, it's a matter of preference, but I prefer logically separate changes to be in separate commits at least within the PR, where it may make reviewing the PR easier. It's basic empathy towards the reviewer(s). Of course, when merged, it often makes sense to squash some commits together, just like you say - long-term, many of the feature development details are not important. It's not an either-or proposition.
Cthulhu_ 5y ago
Small nit: Maybe not say what YOU are doing, but what the COMMIT, when applied, will do. Git is not a personal work log, it's a source code change log. A commit may be a collaborative effort; it may be a squash of a thousand smaller commits; it may be a merge, it may be generated by a tool (like the dependencies), etc. The commit should summarize what IT does, not what YOU did. In ten years, nobody will care who you are or what you did, but they will care what the code does and why.
- MrGilbert 5y ago
  That's the main reason we are using present tense in commit messages, not past tense. It's the commit that is applying the changes at that moment.
  - thaumasiotes 5y ago
    > That's the main reason we are using present tense in commit messages, not past tense. As a linguistic note, commit messages (at least, English commit messages) almost always use an infinitive verb and therefore may be considered to have no tense at all, though to argue that infinitives have no tense you do need to draw a distinction between tense and aspect.
    - timmb 5y ago
      I believe it’s the imperative (enable widget) not the infinitive (to enable widget) that is generally used.
      - thaumasiotes 5y ago
        > I believe it’s the imperative (enable widget) not the infinitive (to enable widget) You're confusing a bare infinitive with an imperative. Infinitives are often, but not always, marked with to. Commit messages are not commands and are not capable of licensing imperative forms. Here are some examples of bare infinitives in English, with the infinitive form italicized: 1. This organization will help you apply for a loan. 2a. Q: What does he do? 2b. A: Check over the paperwork to be sure everything's right. A commit message is doing exactly the same thing as occurs in example 2b.
ignoramous 5y ago
> This blog post is a good description of the most prevalent convention in how to write good and useful messages. See also: https://google.github.io/eng-practices/review/developer/cl-descriptions.html https://google.github.io/eng-practices/review/developer/cl-d... https://microsoft.github.io/code-with-engineering-playbook/code-reviews/pull-requests/ https://microsoft.github.io/code-with-engineering-playbook/c...
jldugger 5y ago
> DEFINITELY say what you are doing. And then say why. My favored github PR template has 3 sections: - What does this PR do? - Why is that valuable? - Link to bug tracker.
- ihaveajob 5y ago
  We have a similar template at work, and since we made it the default (on Bitbucket you can pre-fill it), it's saved a ton of back and forth asking questions: * Description: (What this does, and why) * Ref: (Ticket/task link) * To test: (Specific instructions to make sure things are working as expected) * To deploy: (What specific servers/services to restart, and operations to perform)
zestyping 5y ago
Ironically, the referenced article on how to write good git commit messages (https://cbea.ms/git-commit/ https://cbea.ms/git-commit/) gives no "why" for most of its points. It lists 7 rules, of which 2, 3, 4, 5, 6 are simply declared without rationale.
- pixiemaster 5y ago
  so true. my personal observation: because writing down „what“ is hard. Writing down „why“ is even harder.

pferde 5y ago

Also, if your commit message has parts that say "also, this", "also, that", don't be lazy and break those alsos into separate commits!

jacobmischka 5y ago
Not necessarily. It sounds like all of the things mentioned in the commit message are interconnected (updating build scripts and updating gitignore to remove the new build directories). It would be pointless to break these into separate commits, you can't revert just one without reverting the others. The commit message is lazy, but breaking these into separate commits because of some "also" rule of thumb is just blindly following a rule for the sake of it.
- capableweb 5y ago
  > breaking these into separate commits because of some "also" rule of thumb is just blindly following a rule for the sake of it Agreed, cargo-culting is always bad! But, without knowing how OBS development team works, sometimes it does make sense to split commits like this, mainly if you have a larger repository where different people are responsible for different areas in the codebase. For example, build-scripts sometimes gets managed by other people than the ones writing some core algorithms, who are also different from the people writing public APIs to be consumed by users. In those cases, it makes reviews a lot easier when you can split out to review different commits to different people, instead of having everyone navigate the same commit. But again, not familiar with the internals nor the development cycle that OBS keeps, so maybe this doesn't make sense in this case.
  - threeseed 5y ago
    As others have mentioned many times here. This commit is based off a branch that was adding support for Apple Silicon. So the entire PR is not going to look clean until that branch is merged first.
    - capableweb 5y ago
      Yes, makes a lot of sense. Sorry if I gave the impression that I think somehow this PR is wrong or similar. I was just replying to the fact that sometimes splitting up commits makes sense, outside of this PR.
  - PostThisTooFast 5y ago
- rlpb 5y ago
  My exception to the "also" rule of thumb is that if the split changes would cover separate areas of the code such that each piece could be easily picked out by "git add -p", then splitting them out isn't required as long as they are all associated to one logical larger story. > ...you can't revert just one without reverting the others That's often true in a split out patchset anyway, so I don't think that on its own it stands as a justification for doing things one way or the other.
Cthulhu_ 5y ago
Dubious. I prefer that style myself, but there's a lot of value in squashing multiple commits around a feature into a single, "this commit implements this feature" commit. Is it valuable to retain the entire development history of a feature, or just the feature itself? What if during the development, it was rewritten a dozen times? What if there's a dozen commits where the author meandered and only in the last three commits got to the point? Ruthless interactive rebase is one way to fix that last point, but if you take the final result and squash that into a single changeset, there's a point to be made about that as well.
- pferde 5y ago
  Agreed, it's a matter of preference, but I prefer logically separate changes to be in separate commits at least within the PR, where it may make reviewing the PR easier. It's basic empathy towards the reviewer(s). Of course, when merged, it often makes sense to squash some commits together, just like you say - long-term, many of the feature development details are not important. It's not an either-or proposition.

Cthulhu_ 5y ago

Small nit: Maybe not say what YOU are doing, but what the COMMIT, when applied, will do. Git is not a personal work log, it's a source code change log. A commit may be a collaborative effort; it may be a squash of a thousand smaller commits; it may be a merge, it may be generated by a tool (like the dependencies), etc. The commit should summarize what IT does, not what YOU did. In ten years, nobody will care who you are or what you did, but they will care what the code does and why.

MrGilbert 5y ago
That's the main reason we are using present tense in commit messages, not past tense. It's the commit that is applying the changes at that moment.
- thaumasiotes 5y ago
  > That's the main reason we are using present tense in commit messages, not past tense. As a linguistic note, commit messages (at least, English commit messages) almost always use an infinitive verb and therefore may be considered to have no tense at all, though to argue that infinitives have no tense you do need to draw a distinction between tense and aspect.
  - timmb 5y ago
    I believe it’s the imperative (enable widget) not the infinitive (to enable widget) that is generally used.
    - thaumasiotes 5y ago
      > I believe it’s the imperative (enable widget) not the infinitive (to enable widget) You're confusing a bare infinitive with an imperative. Infinitives are often, but not always, marked with to. Commit messages are not commands and are not capable of licensing imperative forms. Here are some examples of bare infinitives in English, with the infinitive form italicized: 1. This organization will help you apply for a loan. 2a. Q: What does he do? 2b. A: Check over the paperwork to be sure everything's right. A commit message is doing exactly the same thing as occurs in example 2b.

ignoramous 5y ago

> This blog post is a good description of the most prevalent convention in how to write good and useful messages. See also: https://google.github.io/eng-practices/review/developer/cl-descriptions.html https://google.github.io/eng-practices/review/developer/cl-d... https://microsoft.github.io/code-with-engineering-playbook/code-reviews/pull-requests/ https://microsoft.github.io/code-with-engineering-playbook/c...

jldugger 5y ago

> DEFINITELY say what you are doing. And then say why. My favored github PR template has 3 sections: - What does this PR do? - Why is that valuable? - Link to bug tracker.

ihaveajob 5y ago
We have a similar template at work, and since we made it the default (on Bitbucket you can pre-fill it), it's saved a ton of back and forth asking questions: * Description: (What this does, and why) * Ref: (Ticket/task link) * To test: (Specific instructions to make sure things are working as expected) * To deploy: (What specific servers/services to restart, and operations to perform)

zestyping 5y ago

Ironically, the referenced article on how to write good git commit messages (https://cbea.ms/git-commit/ https://cbea.ms/git-commit/) gives no "why" for most of its points. It lists 7 rules, of which 2, 3, 4, 5, 6 are simply declared without rationale.

pixiemaster 5y ago
so true. my personal observation: because writing down „what“ is hard. Writing down „why“ is even harder.