Hacker News Reader

Comment by callum85 | Hacker News Reader

But if the commit message says what happened, then you don't need to read diffs to find out what happened. Reading an English sentence is easier than reading code diffs, and also you can read a bunch of sentences in list form (eg, git log).

I'm not saying you don't need to include the 'why', I'm saying there is value just in the 'what'.

As for code comments that just 'explain something obvious'... I have no problem with comments that simply summarise, in English, what the next few lines do. It means I can skim through a file just reading the comments, until I find the part I want to debug/modify/understand in more detail. Also, if you can sum up some code with a sentence then there's a good chance it makes sense, so it's a good quality check.

View on HN

With the 'what' of course I don't mean 'what happened', as the latter would be actually kind of the 'why' you're making the change. So with the 'what' I mean 'what is being changed', some people just write what file are changing and what they are adding to it, explaining how the code is changing in the diff, but not why the code is changing.

As for code comments, I think you're basically wrong. If you can summarize some fragment of code under a sentence, extract that piece of code into a method and name that method in the same way you were going to write that comment; otherwise comments get outdated easily. These kind of techniques are well explained in a very good book I read a while ago, it's called "Clean Code".

I have no objection to summary comments - they can guard against certain types of accidental or otherwise erroneous edits. If the code and the comment don't quite agree then suspect both of them are wrong.

Personally I think check-in comments should be a concise (but not generic like "bug fix") description of "what" with a smattering of "why" (perhaps a reference to a bug/ticket/feature/request ID). The details of "what" can be seen by looking at the diffs. Where the details of "why" go is split. Why you chose a particular method over others available? Any constants or magic numbers you think might need clarifying when someone else looks? You've done something that looks odd/unnecessary at first glance in order to work around a bug/limitation beyond your immediate control? Those things go in comments in the code. Just about everything else goes in the ticket and possibly makes its way to other documentation, there is not point cluttering the code up too much (unless your implementation documentation is all in-code and you use tools to extract it into other forms, as I've seen done before now, in whic case everything goes in there). "FIXME" and "TODO" markers can go in the code too though generally you want them gone for release (if they survive between releases then they should really be feature requests in your backlog rather than just code markers, so they can be tracked/managed/prioritised as needed.

What really grinds my gears is commits without any comment at all.

> What really grinds my gears is commits without any comment at all.

That just shouldn't be allowed. Commits without JIRA (or whatever you use) task reference should just be automaticaly rejected.

Yeah, the legacy project I'm working on isn't that modern... And even when rules are enforced, people sometimes work around them and leave a useless commit comment. Lax documentation, sometimes deliberately lax, is a person problem unfortuntely - one better solved in ways that HR would disaprove of then via technical blocks!

Ah OK I see what I call "what" you call "why".

"otherwise comments get outdated easily" - not mine. If this is a problem, it's a problem that needs fixing, not a reason to comment less.

"extract that piece of code into a method" Sometimes, yes, but you can overdo this and end up with a fragmented codebase that is harder to follow.

This is an old debate, and there are books on either side. My preference is something approaching Knuth's literate programming.

Reply on news.ycombinator.com