Hacker News Reader

Comment by thom | Hacker News Reader

Somehow I’m really wary of all the energy that goes into source control workflows and commit messages. It’s all meta-information and the only time it really matters is when something has gone catastrophically wrong. If you genuinely have to go looking at the history of a codebase to find out why something weird has happened, that feels like a codebase crying out for a better structure. New features shouldn’t be major surgery, they should be instances of established abstractions. I’m very interested in languages that make that modularity easier but if you can’t delete your entire Git history with no negative consequences that just seems like a disaster to me, on both an individual and industry-wide scale.

View on HN

> the only time it really matters is when something has gone catastrophically wrong

Or in other words, it matters most when it's most needed and the recovery process is glad to have them.

Think about the airline industry and how many times the causes of an accident have been found in the months-old maintenance records of an aircraft, or in an examination of pilot training from last year.

Now imagine throwing away all the information that could have led to finding a cause and putting in place mechanisms and practices to prevent a recurrence. This is how the software industry continues to stumble over the same problems that were known and described in the 70s.

I don’t think it’s _necessary_ when something goes wrong. I think it’s a sign of a process going wrong (which is just as likely as your commit message process going wrong). If you can’t work out why a piece of code looks the way it does or why it even exists, the fix isn’t better commit messages, it’s better code.

I want to be clear I’m not just being a curmudgeon here. There’s been interesting, old work in the OO world about capturing use cases more explicitly in your model. I think some work around micro front-ends is promising. Anything that lets you compose new work in a modular, clear, explicit way is very interesting. And I think that’s the path we should take as an industry, rather than running git blame to understand why a particular case in a switch statement exists. At the very least, everything useful in a commit message could be a comment in the code, but even that seems weak to me.

You're correct that ideally the code would reflect the understanding of the programmers and the business. I've definitely worked with codebases where it's nearly impossible to discover any but the most basic things about the business, and I appreciate better code. Still, even if the code is clear, it's clear about how things are now, but what about in the past? Can just looking at the differences between and now clarify why things changed, and why the code was changed the way it was? What's become sort of a mantra for me, "what problem were they trying to solve?" That's not often clear in the diffs.

The ‘why’ of all of your code at the macro level should be clear from the acceptance tests (which hopefully document the motivating use cases and aren’t just barren series of steps with enigmatic names). At the micro level it should be done with naming (it’s really good to have the strategy pattern here because you can explicitly capture policies as named things, and occasionally keep old ones around as a contrast) and comments or doc strings.

I don’t know why the industry has settled on the idea that we only talk about ‘why’ at diff time in external tools. Even if the tools and languages today aren’t ideal, I think it’s a worthwhile thought experiment to constantly ask yourself, how can we better capture this information and serve this workflow in one place?

For example, most codebases put everything in a series of functional siloes based on the architectural layering of the app. What would it take for a framework to make it easy to write code built around vertical _features_, all the code for each of which is co-located (including tests and documentation), and which integrate into the app via various hooks. There are obviously a million ways to poke holes in this idea but I think we’ve learned the lesson in the industry a thousand times over that functional siloes are bad from a team structure and work scheduling point of view. Perhaps that ought to also apply all the way down to the program text.

Ultimately I’m more intrigued than religious here, but I will maintain that it’s at least a code smell when someone has to care too much about commit messages.

The very obvious place where this is useful is when seeing a random `if` statement and wondering why it's there. Git can pull up the commit it was added in, and the right context.

This can also quickly identify who added it (so you can ask someone "hey, if I changed this to that would that make sense in your opinion?")

Like with many things in life it's not a guarantee of exactitude or rightness, but it can be very helpful information!

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?

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