Hacker News Reader

Comment by thom | Hacker News Reader

But we can both accept that the meaning not being clear in the code is bad, right? And the ideal solution would be the code being clearer, not the commit messages being clearer? I just think I’d always put my effort into the code and incentivise that over everything else. I mean, if you have a highly involved Git workflow already, aren’t you doing code reviews and catching this stuff anyway?

View on HN

That is a legitimate point.

One challenge with this is that in a code review you have the context of the change in front of you, and are looking at changes. Many changes are "obvious" when seen in the context of other changes. But later on people aren't looking at changesets, but final artifacts! That difference in perspective can be hard to decipher.

Now of course there are things that are "obviously this should be commented clearly". But then there are more subtle things that make sense to the person writing the code, and the handful of reviewers, just because they have experience in a domain. Someone else comes in and it just isn't clear to them for... no fault of anyone really. But the clean commits can help this person discover answers more quickly than if there is no VCS information at all.

I think this might be the clarity equivalent of "defense in depth". You want things to be clear, but sometimes things are not that straightforward and you gotta dig in a bit more.

On a more holistic level as a commit writer, I'm generally only making a handful of commits in the end (I like squashing up the fixups and the like). I write out commit messages, and usually those messages become the PR messages (with some edits as needed). That process helps me clean up code and add comments as well. The process of making this stuff easy to understand involves thinking about it multiple times in the process from thought to committed code!

So yes, make the code clear. Sometimes things are hard to make unambiguously clear from any angle, though, so also make the changelog clear!

Okay so we’re learning even more here - code reviews have some flaws which perhaps we can address by performing smaller more isolated reviews first, or at least spending more time on the question of whether code makes sense in isolation.

I accept it’s hard to make code unambiguously clear sometimes but not that it’s impossible - commit messages are just text after all. After having to look up the commit messages do we at least make changes _then_? Do we try to learn a lesson to avoid the need in the future? If someone banned you from ever writing commit messages again, are you sure you’d make no changes to the way you write code? And if you would, why not make those changes today?

Sometimes the code isn't clear because the reason for the "if" has nothing to do with the code and everything to do with externalities.

For example, I wrote a DNS server that had to take into account that on many Linux systemd distributions a daemon `systemd-resolve` binds to 127.0.0.54:53, preventing my DNS server from binding to INADDR_ANY port 53.

So my DNS server has a contorted piece of code that, when binding to INADDR_ANY fails, iterates through all the interfaces and all the IP addresses and binds to each one individually.

Anybody looking at the code would rightfully ask, "What the heck?", but if they read the commit messages, the reason becomes clear.

Why not just have a comment? Or structure the code such that the initial failure calling your bind_to_in_addr_any function sets a flag called inaddr_any_already_bound or something and checking that variable calls another function called bind_to_available_interfaces? There is nothing that can be explained in a commit message (which is just text) that can’t be explained in code (which is just text).

Proper naming of variables/functions is important, but would only tell a part of the story. In the example you'd loose the context why that port might be already in use.

Using comments instead of commit messages is of course always possible, but if you'd note all relevant information you'd otherwise put into comment messages in comments, you might end up with more comments than actual code, which IMO hinders readability significantly. Take the Linux kernel for example. git commit messages there are very informative and often pretty long. Putting this information into comments would make the code pretty hard to read. Also sometimes there isn't a clear place where you'd put such a comment at, as it only makes sense when looking at multiple changes in different locations (= a git commit) at once.

I believe having the explanation why certain aspects of the code are as they are in the metadata in form of the git commit messages with an occasional comment in the code for important information is a good compromise between readability/verbosity of the code and having all relevant context available.

Reply on news.ycombinator.com