Don't rely solely on your code base's latest snapshot. Rely on its history. | Hacker News Reader

Don't rely solely on your code base's latest snapshot. Rely on its history.

blog.ezyang.com

28 pointstimn10 comments

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

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.

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.

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.

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

In your ~/.gitconfig:

    [alias]
      ci = commit -v

Then always commit with: git ci

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.

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

Reply on news.ycombinator.com