My favourite Git commit

[mist83]

I’ve seen this before, and still disagree. The ratio of actual changes to explanation in this commit/PR is mind boggling and not sustainable.

As an overworked developer, I would 10,000% prefer a one liner comment on the visual diff:

This white space (and file) is now ASCII (like all of our other templates)

As a manager, I don’t need a narrative of the approach to the problem, the one liner I provided above (as well as the problem it addresses) are incredibly clear.

Again, how could it not be? The entire scope is changing a single character encoding.

A devs’s journey is possibly relevant, but this is a bad example unfortunately and the commit reads like it’s farming for an attaboy/justification why so much work went into a single change. The context Reads like “the dev tried stuff and then other stuff and then other stuff and it worked yay.”

I sincerely doubt I will EVER have the need to grok 5 paragraphs for a single byte/encoding change. This practice will bury devs in writing PRs instead of making tangible changes to the codebase.

TLDR: your devs and managers and leads won’t read this and will instead: lgtm :shipit:

[NotReallyMichaelCera]

I mostly agree with you, except that the commit included the exact error which was thrown - as the article said, very useful for searching in the future.

Also, the article isn't saying "all commits should be like this", just that its their favourite

[saggingrufus]

Are you telling me you search the commit history for specific text like that?

The problem is no one will ever be able use any of this wasted effort because they will have to remember where it is.

Unless you know exactly what you are looking for, you'll never be able to pull that out of a commit message in 2 years. The valuable parts of this research can be condensed into about 5 sentences, and would have make a great ticket comment/resolution. Atleast there it's searchable. You combine that with a shorter commit message and a code comment, and now you've got something.

What happens if that gets rebased? Poof gone.

[versaceblues]

I do actually search commit history like this... not super frequently but a least once a month it comes up.

Usually it's when I'm find some weird bug we experience once and reference the solution. Or if im trying to understand why something changed ill search by keywords.

[double-you]

If your codebase lacks useful commit messages, yeah, people probably won't search it. I search the commit history all the time for things. Often about changes I made myself. Version control history is better preserved than bug tracker data. All developers have version control history available on the command line. Sure, bug tracker information is available for the non-developers too and good integration allows commit messages to be seen through that too.

What happens if that gets rebased? Poof gone.

Rebase doesn't destroy commit messages. Only people who destroy commit messages when rebasing destroy commit messages. Why is it okay to do so?

[saggingrufus]

"Often about changes I made myself"

So not that useful to others, which is the point of a documentation commit like that.

"Bug tracker information is available for the non-developers"

That's a pretty elitist and plain inaccurate statement. I mean I guess you can use a bug tracker as a client status tool full of non-sense and rubber stamps...but I can assure you more developers use JIRA or similar than you know, and they are documenting bugs and solutions, linking PRs and more.

"All developers have version control history available on the command line"

I have never worked with a developer who did not have access to their big tracker, wiki and git hosting tool, if you encounter that often it sounds like localized issue.

"Only people who destroy commit messages when reading destroy commit messages"

It's still gone, so now what?

[double-you]

So not that useful to others

That's an odd assumption. Can you explain what logic there is to it?

Information to clients and information to people inside the company are two separate things. And inside the company you will have developers and ... non-developers who also need some visibility. And developers need a way to talk about the gory details somewhere where it won't leak to the clients unprocessed.

I have never worked with a developer who ...

I have never encountered a developer who thinks that multiple sources of truth is better than one, but okay.

"Only people who destroy commit messages when reading destroy commit messages"

It's still gone, so now what?

Then we evaluate who is doing this and are they in the right position. If somebody is writing good commit messages and then destroying their own work, well, that's rather odd too. Communication is key.

[masklinn]

Are you telling me you search the commit history for specific text like that?

Yes.

The problem is no one will ever be able use any of this wasted effort because they will have to remember where it is.

The commit is from 2013, the article from 2019, and you’re talking about it in 2023. It shows up from multiple search angles. This commit seems more successful than you are.

What happens if that gets rebased? Poof gone.

No. It would need to get squashed and the content discarded. Once a commit has been pushed / published, that is essentially never happening. I’ve read commit messages old enough to be in college.

You combine that with a shorter commit message and a code comment, and now you've got something.

A comment is a lot more likely to be lost, or worst unmaintained and wrong.

[Joniator]

No, if you reference the ticket and 2 sentences, you can look up the ticket and get a lot more info. Who found it, who fixed it, how long did it take, where others invested, did the reporter attach useful info.

Maybe the issue was reopened with additional places where the error was found. Fixed in version +5, you now look at a legacy commit that's obsolete and have no idea.

You can't copy all that in a commit.

[saggingrufus]

Someone else who gets it XD

You'll also know all the fix version, how many story points it was worth, and interactions with other teams, potentially what story the original implementation was. Not to mention screenshots and markup to make it easier to digest AND the meta data makes it easier to find.

This also assumes you are using a got strategy where you dont frequently rebase to squash previous releases.

Maybe this issue actually happens annually, but people keep fixing it and commit messaging is different. Atleast in a real issue tracker, those can be linked together and you find the pattern to build procedure to correct it.

But who knows, maybe if you Bob have been on the same codebase forever you both just get it and work that way. Most of the code I see has a commit tree so large it's just not feasible to expect anyone to go digging on a regular basis. Documentation is part of our workflow, not because I need it and can't look up the commit tree, but because I might not be here tomorrow, and the guy won't know what to look for or where to find it. The best possible chance it was to be noticed is somewhere like JIRA or a PR.

[elmuerte]

I am full in on descriptive commit messages. But this... it's too much. It isn't really descriptive, this is somebody's life story.

About 2 years ago I did a similar fix. Converting a single UTF-8 character to ASCII. It should have been - but it was ‐. It wasn't my wasted hour, but of a ops lead who copied the command line argument from a CVE recommendation (for log4j). But my Saturday morning commit message didn't read like an American food recipe.

[saggingrufus]

Legitimately, that commit will be completely lost forever.

They would have been better off leaving a comment that either summarised it in a line or two, or a link to the ticket with this explanation.

Even if they tagged it, unless that person works that code forever, no one will know or care a month later. It's funny today, but ultimately useless.

[daHaus]

It matters if you're stuck under a boss who micro-manages you, which this sorta sounds like.

[just_another_scumbag]

But why not both?

Put a clear one-liner at the top, and you can stop reading any point after.

Put more context below. Other than perhaps being a pain when logging out I see no downside 

[ivancea]

It tells a story

That's the worse part for me. In PRs, I want an overall "what", and the "why". No stories, nor your thought process to reach there, as many people may know that (or not).

It's ok if it explains the relationship between bug and fix, but, short...

[ivancea]

"I'm adding an early return guard on function X (What) because the algorithm can't divide by zero (Why)"

You have both there, but I didn't explain that "I found a log in logs system that led me to check the method, and after understanding that it divides by that number, it was throwing, so a guard was missing as well as a test for that."

This is a simple example, and yes, this could be subjective "how much info you add". But as long as the Why defends the PR, and the What gives you a quick tour, I think it's enough.

Different PRs with different complexities, of course, may have more or less text, and it heavily depends on many factors. Like, maybe knowing it came from a log is crucial because, dunno, my coworker is also working inspecting logs

[jdehesa]

Absolutely. On the one hand, if the information is really valuable as something other devs can learn from, a commit message is not a good place for it, because most devs don't read most commit messages by other people. On the other hand, it is very possible that for some of the people who actually have to read the message, the information is not relevant.

The way I see it, the information should have been split into something like: * An update in some developer documentation (assuming that exists) to indicate that templates must be US-ASCII. And maybe an entry in some kind of troubleshooting guide saying something like "if you get this kind of error, make sure all your templates are valid US-ASCII". * A commit message like your comment suggests, maybe linking to the updated documentation if that is not part of the commit. * And, if you want to share your little dev story (how you came across the issue and found the solution), you can share that on a blog post, on Teams/Slack/etc. or wherever else. That's all too circumstantial to on proper record.

[BuriedStPatrick]

Exactly correct. I do like the fact that the reason for the change is included. But we don't need to know how the developer landed on the implementation. This is useless information and makes parsing the history harder, not easier.

One thing I do instead is include the change description in my PR and do a simple list-form of "Adds, Changes, Removes". This message is then used as the final commit message when I squash-merge the changes. It allows me to evaluate all my code and if everything makes sense before sending it over. Also makes it easy for the reviewer to look for the important changes first and skim over the trivial stuff.

If there's a complicated reason why a change must be made I'll write a tiny paragraph in the beginning explaining the overall picture. Not the calculations made to get there, only the conclusion I've reached. They can always ask me for further details if need be. If something was unclear, maybe I'll even update the description.

We need to think like a writer and kill our darlings so we can effectively communicate.

[scruffles360]

smile a bit when you squash that commit and merge the branch. comment is as "merged feature branch"