Don't rely solely on your code base's latest snapshot. Rely on its history.
1–10 of 11 posts
Re: Don't rely solely on your code base's latest snapshot. Rely on its history.
#2This is also another benefit of having an organized code-review system that records review comments perpetually. Oftentimes, I'll wonder why someone's using an obviously short-sighted and naive algorithm, only to look through the review comments and see the original author say "I know this is a bit of a hack, but it gets the job done now, we've got to check this in in the next two hours, and we can fix it later." Or I'll do the same and see, "I tried this elegant solution, but it doesn't work for reasons X, Y, and Z", saving me many hours of going down a blind alley.
It's generally good software engineering practice to build for your needs now and not worry too much about anticipating your needs later. Code reviews naturally end up surfacing lots of concerns of the form "But what happens if the system changes in this way later?" If you have a record of that, then when the system actually does change that way later, you can refer back to what the issues were and what potential solutions the original designers had in mind.
Re: Don't rely solely on your code base's latest snapshot. Rely on its history.
#3Comments with every commit are definitely important and helpful. And they have to be done while looking at the diff.
The practice of reviewing and commenting diff not only helps in future maintenance, it also has immediate benefit -- help to identify bugs and silly mistakes in current commit.
I don't use Git though -- I use Subversion.
Re: Don't rely solely on your code base's latest snapshot. Rely on its history.
#4This is also another benefit of having an organized code-review system that records review comments perpetually. Oftentimes, I'll wonder why someone's using an obviously short-sighted and naive algorithm, only to look through the review comments and see the original author say "I know this is a bit of a hack, but it gets the job done now, we've got to check this in in the next two hours, and we can fix it later." Or…
The example you gave should rather go into the code as a code comment. Esp. if it is only a hack, it is much better to give such a statement there instead of the commit comment.
The commit comment should more be about what you have worked on and/or what you have completed or so.
Btw., for a small team, the code comments itself may also be a good place to write the review comments into, maybe marked with a TODO or so.
In both cases, you have the information right there where you may need it.
Re: Don't rely solely on your code base's latest snapshot. Rely on its history.
#5> never write a commit message without the diff in hand
To do this in git, use the --verbose flag with git commit. I use it in my configuration by default, and it really helps.
Re: Don't rely solely on your code base's latest snapshot. Rely on its history.
#6This is also another benefit of having an organized code-review system that records review comments perpetually. Oftentimes, I'll wonder why someone's using an obviously short-sighted and naive algorithm, only to look through the review comments and see the original author say "I know this is a bit of a hack, but it gets the job done now, we've got to check this in in the next two hours, and we can fix it later." Or…
The example you gave should rather go into the code as a code comment. Esp. if it is only a hack, it is much better to give such a statement there instead of the commit comment. The commit comment should more be about what you have worked on and/or what you have completed or so. Btw., for a small team, the code comments itself may also be a good place to write the review comments into, maybe marked with a TODO or so.…
Code comments are best for spatially local concerns. Commit messages are best for temporally local concerns. Design decisions are temporally local concerns. "What if _" is something you just thought up today. Tomorrow you'll be concerned about other issues. A long code comment about something you don't care about will keep getting in the way.
Re: Don't rely solely on your code base's latest snapshot. Rely on its history.
#7This is also another benefit of having an organized code-review system that records review comments perpetually. Oftentimes, I'll wonder why someone's using an obviously short-sighted and naive algorithm, only to look through the review comments and see the original author say "I know this is a bit of a hack, but it gets the job done now, we've got to check this in in the next two hours, and we can fix it later." Or…
The example you gave should rather go into the code as a code comment. Esp. if it is only a hack, it is much better to give such a statement there instead of the commit comment. The commit comment should more be about what you have worked on and/or what you have completed or so. Btw., for a small team, the code comments itself may also be a good place to write the review comments into, maybe marked with a TODO or so.…
If every design discussion we had in the review comments ended up as a comment in the code, the comments would be 10x longer than the code. There's one CL that I'm currently reviewing where the review thread is 41 messages long. That's not all that atypical for changes that require any sort of design discussion.
Though yeah, something that's a quick hack should have a " TODO(username): Fix hack. See review discussion in CL 12345678." to let the reader know that there's something else they should be aware of.
Basically, I think the code comments should say what the code does (and be rather sparse, usually, because you should be able to tell from the code itself), the commit comment should say what the change does, and the reviewlog should say what the code doesn't, i.e. roads not taken, design alternatives considered and rejected, tradeoffs made. The code always doesn't do far more than it does; putting that in the code comments makes it harder to follow what the code does do.
Re: Don't rely solely on your code base's latest snapshot. Rely on its history.
#8> never write a commit message without the diff in hand To do this in git, use the --verbose flag with git commit. I use it in my configuration by default, and it really helps.
How do you set this by default? I looked through git-config, and elsewhere, but I couldn't find any way to do it, short of using a shell wrapper.
Re: Don't rely solely on your code base's latest snapshot. Rely on its history.
#9> never write a commit message without the diff in hand To do this in git, use the --verbose flag with git commit. I use it in my configuration by default, and it really helps.
How do you set this by default? I looked through git-config, and elsewhere, but I couldn't find any way to do it, short of using a shell wrapper.
you can probably do something like shadow the commit command with an alias in your global pref file
Re: Don't rely solely on your code base's latest snapshot. Rely on its history.
#10Earlier quoted context omitted.
How do you set this by default? I looked through git-config, and elsewhere, but I couldn't find any way to do it, short of using a shell wrapper.
you can probably do something like shadow the commit command with an alias in your global pref file
git (sensibly so) refuses to permit aliases like that.