5 ms·

> I don't want your entire life story in my commit log. I[1] want enough debug information in the commit log to be able to reproduce the issue without having t

by leejo 7y ago

> I don't want your entire life story in my commit log. I[1] want enough debug information in the commit log to be able to reproduce the issue without having to go on web hunts to understand the problem. Especially when the change appears to be trivial on the surface, because these are the ones that can turn out to be rabbit holes. I don't want to have to interrupt you to get this information because you didn't write a good enough commit message, and you probably don't remember anyway. I don't want go look at an external issue tracker that i may not have access to, or may not even exist anymore. [1] Where "I" is: me, your future self, a future maintainer, a junior dev, an open source contributor.

afiori 7y ago
> I don't want go look at an external issue tracker Related question: are there projects that use git itself as issue tracker?
- johnday 7y ago
  I can imagine that working to a degree: Make a fork of a commit at an issue, then merge that fork back in with master at point of fix. Bit of a mess in the tree though.
- _TwoFinger 7y ago
  https://news.ycombinator.com/item?id=13732598 https://news.ycombinator.com/item?id=13732598
- cipherboy 7y ago
  Pagure [0], Fedora's git forge, hosts code, issues, docs, and pull requests as four separate git repositories under the hood [1]. However, only project administrators can clone most of those repos. [0]: https://pagure.io/pagure https://pagure.io/pagure [1]: https://docs.pagure.org/pagure/usage.html https://docs.pagure.org/pagure/usage.html
- BlueTemplar 7y ago
  https://github.com/MichaelMure/git-bug https://github.com/MichaelMure/git-bug
cmroanirgo 7y ago
≥ Especially when the change appears to be trivial on the surface Comments about the code should be in the code, where the next dev will see it. The more trivial a change, with far- reaching implications, the more important this is. Doing so has heaps of benefits: future devs understand ramifications, shows that this code has been scrutinized, makes it easier when doing refactoring /yanking, or porting code. That said, leaving the life story out will always be a good idea.
- schnable 7y ago
  Agreed, but in this case, it was an encoding/whitespace change so there isn't really anywhere else to put this info.
- bluedino 7y ago
  IMO the repo is the code However, I would have done a simpler commit and linked to an issue where I explained the problem/solution in more detail
  - mark-r 7y ago
    This assumes your issue tracker doesn't change. I've been at my current position 8 years, and in that time we've had 3, and the first 2 are shut down.
    - bluedino 7y ago
      We use Gitlab so it has both in the same project - but like you said that could change
AstralStorm 7y ago
Write a regression test (including its documentation) instead of just documenting the issue in human interpreted language, immutably. Your future maintainer will thank you for not having to dig through repository history.
- leejo 7y ago
  It goes without saying that commits should include tests that cover the change, where possible. > immutably That's what makes this modus operandi so powerful IMO - comments in code may go unmaintained, tests may start failing for other reasons, issue trackers come and go, developers leave the company, documentation rots. The commit message is (unless you have a bad actor) immutably linked to the original change, and that's exactly why you should be thorough in expressing its reason for being. I can git checkout the point in time (perhaps having bisected) and have the information to allow me to reproduce the issue.
gus_massa 7y ago
I prefer that the commit includes the addition of a test in the test suit that get fixed (or a few). This is good because: * It ensure that the bug is real. [1] * It ensures that the bug is fixed. [1] * It prevents reversions (assuming the test are run automatically). * The test may prevent reversions in other related code, or discover other hidden bugs. * It brings you closer to a 100% test coverage. * You don't have to guess how to reproduce the bug, reading the comment. * If the bug depends on subtle configurations, they should be set in the test. [2] From time to time there are bugs that are obvious in the code, but they are too difficult to find a test for them. [1] Been there, done that. [2] Once I found a bug that depended on the local timezone.
- toastal 7y ago
  100% test coverage is such an overrated stat.
MrGilbert 7y ago
This. It's the same with comments in code: I don't want to read what the code does (I can read that myself, thanks!), I want to know WHY it does it the way it does it - especially, if there is a more obvious, better way. Also: People leave companies. Or die. At some point in time, you won't be able to ask the original author.
- throwaway40324 7y ago
  No thanks. If I had a dime for every function that's so obviously self-documenting to what it does.. etc. If you tell others what it does and why, concisely and thoughtfully, no one has to try and mentally parse the what of some clever undescriptive block of code.
0zymandiass 7y ago
To me, at least, the issue with that commit message is the signal-to-noise ratio. There is a lot of exposition for each piece of information. I prefer a more declarative commit message. However, from the writing, I suspect this is just due to the committer not being a native English speaker. e.g. the first paragraph doesn't lose any important information trimming it down to: "After adding a test matching the contents of router_routes.conf, `bundle exec rake` fails with: ArgumentError: invalid byte sequence in US-ASCII " Realistically, it would have been a better commit message if they'd given the shortlog SHA where the test was added that exposed the bug rather than an explanation of what the test does. "After adding test <testname> (08c3e17), `bundle exec rake` fails with:"
- robin_reala 7y ago
  Having worked with the committer, I can tell you that he’s definitely a native English speaker.
- SamBam 7y ago
  Hmmm, I've always believed that no commit should break a build, even if you're committing the fix right after. Otherwise you're going to cause problems for `git bisect` or other practices of going through the history to find where a problem may have started. Do other people commit breaking tests and then fixes?
  - Bartweiss 7y ago
    I think this depends quite a bit on what other contributors are doing - it's one of those cases where several approaches are acceptable, but inconsistency is not. "Commits should always build" is one doctrine I've seen. As you say, it makes bisecting and other error-analysis approaches easy. On the other hand, it risks either having large, opaque commits, or adding overhead to make intermediate commits build - possibly with flawed/meaningless behavior when they do. Another is "the trunk should always build". In that case, you'd just squash branch commits down to logical groupings that are easy to analyze, whether or not things build. You can bisect on the trunk, but lose all guarantees about state on branches. Finally, I've seen variations on "no commits that break the product", "no commits that make things worse", or "no committing failing tests without subsequent fixes". In this case, you can't generally commit broken builds, but can specifically add failing tests. The first rule just means "adding failing tests is ok", the second means "converting runtime bugs to failing tests is ok", and the third means "write your test and fix, but split (and ideally tag) the commits". All of these break bisect, but they guarantee the project itself won't become more broken from commit to commit, and they can help with other forms of reasoning about where bugs first occurred. Every approach there seems viable if you stick to it. If there's no established practice, I suppose the best choice would be based on what sort of work and debugging is most likely to apply.
Groxx 7y ago
"You spent an enormous amount of time learning X, which is encoded in this three-letter bugfix. Don't make the next person go through that too."

by leejo 7y ago

afiori 7y ago

> I don't want go look at an external issue tracker Related question: are there projects that use git itself as issue tracker?

johnday 7y ago
I can imagine that working to a degree: Make a fork of a commit at an issue, then merge that fork back in with master at point of fix. Bit of a mess in the tree though.
_TwoFinger 7y ago
https://news.ycombinator.com/item?id=13732598 https://news.ycombinator.com/item?id=13732598
cipherboy 7y ago
Pagure [0], Fedora's git forge, hosts code, issues, docs, and pull requests as four separate git repositories under the hood [1]. However, only project administrators can clone most of those repos. [0]: https://pagure.io/pagure https://pagure.io/pagure [1]: https://docs.pagure.org/pagure/usage.html https://docs.pagure.org/pagure/usage.html
BlueTemplar 7y ago
https://github.com/MichaelMure/git-bug https://github.com/MichaelMure/git-bug

cmroanirgo 7y ago

≥ Especially when the change appears to be trivial on the surface Comments about the code should be in the code, where the next dev will see it. The more trivial a change, with far- reaching implications, the more important this is. Doing so has heaps of benefits: future devs understand ramifications, shows that this code has been scrutinized, makes it easier when doing refactoring /yanking, or porting code. That said, leaving the life story out will always be a good idea.

schnable 7y ago
Agreed, but in this case, it was an encoding/whitespace change so there isn't really anywhere else to put this info.
bluedino 7y ago
IMO the repo is the code However, I would have done a simpler commit and linked to an issue where I explained the problem/solution in more detail
- mark-r 7y ago
  This assumes your issue tracker doesn't change. I've been at my current position 8 years, and in that time we've had 3, and the first 2 are shut down.
  - bluedino 7y ago
    We use Gitlab so it has both in the same project - but like you said that could change

AstralStorm 7y ago

Write a regression test (including its documentation) instead of just documenting the issue in human interpreted language, immutably. Your future maintainer will thank you for not having to dig through repository history.

leejo 7y ago
It goes without saying that commits should include tests that cover the change, where possible. > immutably That's what makes this modus operandi so powerful IMO - comments in code may go unmaintained, tests may start failing for other reasons, issue trackers come and go, developers leave the company, documentation rots. The commit message is (unless you have a bad actor) immutably linked to the original change, and that's exactly why you should be thorough in expressing its reason for being. I can git checkout the point in time (perhaps having bisected) and have the information to allow me to reproduce the issue.

gus_massa 7y ago

I prefer that the commit includes the addition of a test in the test suit that get fixed (or a few). This is good because: * It ensure that the bug is real. [1] * It ensures that the bug is fixed. [1] * It prevents reversions (assuming the test are run automatically). * The test may prevent reversions in other related code, or discover other hidden bugs. * It brings you closer to a 100% test coverage. * You don't have to guess how to reproduce the bug, reading the comment. * If the bug depends on subtle configurations, they should be set in the test. [2] From time to time there are bugs that are obvious in the code, but they are too difficult to find a test for them. [1] Been there, done that. [2] Once I found a bug that depended on the local timezone.

toastal 7y ago
100% test coverage is such an overrated stat.

MrGilbert 7y ago

This. It's the same with comments in code: I don't want to read what the code does (I can read that myself, thanks!), I want to know WHY it does it the way it does it - especially, if there is a more obvious, better way. Also: People leave companies. Or die. At some point in time, you won't be able to ask the original author.

throwaway40324 7y ago
No thanks. If I had a dime for every function that's so obviously self-documenting to what it does.. etc. If you tell others what it does and why, concisely and thoughtfully, no one has to try and mentally parse the what of some clever undescriptive block of code.

0zymandiass 7y ago

To me, at least, the issue with that commit message is the signal-to-noise ratio. There is a lot of exposition for each piece of information. I prefer a more declarative commit message. However, from the writing, I suspect this is just due to the committer not being a native English speaker. e.g. the first paragraph doesn't lose any important information trimming it down to: "After adding a test matching the contents of router_routes.conf, `bundle exec rake` fails with: ArgumentError: invalid byte sequence in US-ASCII " Realistically, it would have been a better commit message if they'd given the shortlog SHA where the test was added that exposed the bug rather than an explanation of what the test does. "After adding test <testname> (08c3e17), `bundle exec rake` fails with:"

robin_reala 7y ago
Having worked with the committer, I can tell you that he’s definitely a native English speaker.
SamBam 7y ago
Hmmm, I've always believed that no commit should break a build, even if you're committing the fix right after. Otherwise you're going to cause problems for `git bisect` or other practices of going through the history to find where a problem may have started. Do other people commit breaking tests and then fixes?
- Bartweiss 7y ago
  I think this depends quite a bit on what other contributors are doing - it's one of those cases where several approaches are acceptable, but inconsistency is not. "Commits should always build" is one doctrine I've seen. As you say, it makes bisecting and other error-analysis approaches easy. On the other hand, it risks either having large, opaque commits, or adding overhead to make intermediate commits build - possibly with flawed/meaningless behavior when they do. Another is "the trunk should always build". In that case, you'd just squash branch commits down to logical groupings that are easy to analyze, whether or not things build. You can bisect on the trunk, but lose all guarantees about state on branches. Finally, I've seen variations on "no commits that break the product", "no commits that make things worse", or "no committing failing tests without subsequent fixes". In this case, you can't generally commit broken builds, but can specifically add failing tests. The first rule just means "adding failing tests is ok", the second means "converting runtime bugs to failing tests is ok", and the third means "write your test and fix, but split (and ideally tag) the commits". All of these break bisect, but they guarantee the project itself won't become more broken from commit to commit, and they can help with other forms of reasoning about where bugs first occurred. Every approach there seems viable if you stick to it. If there's no established practice, I suppose the best choice would be based on what sort of work and debugging is most likely to apply.

Groxx 7y ago

"You spent an enormous amount of time learning X, which is encoded in this three-letter bugfix. Don't make the next person go through that too."