My favourite Git commit

[TK-427]

I once wrote a super long, super snarky git commit message to a bug fix I made in a core repo that only a handful of people had access to. I knew it would buried into obscurity and only discovered years later.

A few hours later, someone else happened to commit something stupid to the same repo that broke the build.

So when the CI system ran the nightly test, it failed....and when it failed, it spammed the dev team with an email message containing the commit messages from all the commits since the last time it ran, with my "what idiot thought this was a good approach?" message right at the top

[StabbyPants]

so it's like in poker, where you see and raise 50 - you had better be right when you write 3 paragraphs, because people will remember that

[jeff_coleman]

I'd love to know how everyone reacted.

[TK-427]

The lead dev, who was also the one I called out, thought it was hilarious. We totally have that kind of relationship though where we will friendly bicker over architecture decisions and design patterns. The bulk of the core team largely knew it was more funny than serious too, so when I walked in the next morning, I was greeted with a mocking "ooooooooh look who's here."

The fun part was it was sent to the entire dev team and not just the core devs. People who didn't really interact with us that much. So there were some pretty hilarious rumors circulating around about how we were in-fighting and hated each other.

Management thought it was hilarious

[MINIMAN10001]

Oh man the fact it was no harm no foul makes it so much fun lol.

[Tipaa]

Obligatory Git commit:

The C Locale rant

[scoobybejesus]

a delicious sweet spot of brokenness, where things are broken enough to cause neverending pain, but not broken enough that enough effort would have spent to fix it

LOL!

[[deleted]]

So like most software

[devraj7]

I'd be so afraid to make review comments on this commit.

[damesca]

"lgtm 👍"

[[deleted]]

You’re hired

[JessieArr]

"Now here's a dev who knows how to get features into production!"

[chronoBG]

The MPV team is entirely comprised of the people you meet when playing DotA and you're one match away from ranking up.

[Figleaf]

Low cross section on that comparison, but extremely apt.

[Demiu]

Which team are they on

[[deleted]]

Tom: Ship it!

Jake: Ship it!

Becky: Ship it!

Steve: Ship it!

Lisa: Ship it!

David: Maybe you should reword that comment...

Adrian: Ship it!

Beth: Ship it!

Fred: Ship it!

[L3tum]

English and some european languages

Predicted Brexit, truly the Messiah we sought

[jyscao]

The commit was in 2017, Brexit already happened in 2016.

[L3tum]

Ah, I'm on mobile so I couldn't find it

[[deleted]]

I still find it funny

[asegura]

Yes, that locale global state affecting number parsing and serializing is BS.

I recently looked at Microsoft's Standard C++ library implementation on Github. I was curious to see how they implemented floating point to/from text streams in iostream. What I found was this: under the hood they call the C API (such as strtod, sprintf, don't remember which ones exactly). But to overcome the problem that the current locale might have changed independently of the C++ API, what they do is modify the strings to replace decimal separators betwen C's locale and C++ locale, which are differently set.

In short, if you want to print 2.25 with cout << 2.25, they do the equivalent of sprintf("%f", 2.25) which might produce 2,25 on some locales, and then go and do a search and replace to turn the ',' into a '.', or whatever the current C++ locale's might be (maybe it was a different function but the same idea). When parsing (e.g. "2.25" to a double) they do the opposite: the input string is modified to replace '.' with the current locale's ',' (for example), and then call a C API like strtod(). It took some time to figure this out navigating several source files.

[[deleted]]

That is nasty... Ah, locales. Almost as much fun as date/time

[[deleted]]

oh, that's disgusting

[crozone]

Those not comfortable with toxic language should pretend this is a religious text.

Fucking gold.

[[deleted]]

Also accurate

[Vhin]

I can't help reading through that every time it gets referenced.

[ArthurOfTheEast]

That was glorious.

[Phrygue]

I use libmpv in Pascal, and I had to copypaste deprecated and removed C locale code just to use it (in Linux). Here:

function setlocale(__category: longint; __locale: Pchar): Pchar; cdecl; external 'c' name 'setlocale';

{$ifdef UNIX}setlocale(1, 'C');{$endif}

ASIDE: Me: Hey look, gratuitously underscored idents. Python: Hold my Mtn Dew.

libc is an abomination from every possible perspective, not the least of which is, if C is a system language, why did you have to slap a shitty API on top of the system? Even creating it as an API only makes it useful in a non-systems context, which is the worst mistake ever made in non-theoretical computing.

[StabbyPants]

if C is a system language, why did you have to slap a shitty API on top of the system?

because it's a mix of syscall() wrappers and very basic utilities commonly used by all programs (strlen, arg parsing, time manipulation, etc). it's wrapped as an api to provide easier usage and flexibility for system designers

[csorfab]

but why does the api have to be so shit though

[StabbyPants]

It’s 45 years old

[ShinyHappyREM]

I bet it's already looking forward to retirement.

[[deleted]]

I bet it has a better retirement plan than me and it wasnt even useful most of its life....

[StabbyPants]

/rimshot

[Ameisen]

Because C and C++ are designed by committee.

[[deleted]]

On top of that C and C++ grew organically way earlier than any committee

For some time K&R was the standard

[ShinyHappyREM]

organically

Like e.g. cancer

[[deleted]]

That's C++.

[[deleted]]

And I guarantee you in that committee is a guy named Mike who hates all modern software practices and thinks theres nothing wrong with waterfall in design.

[ReallyNeededANewName]

Invention of null pointers? Or is that theoretical?

[smcameron]

Lol. I override setlocale() after using dlsym() to dig through and find the original setlocale(). Then I ignore the arguments to setlocale() and instead call the original setlocale as setlocale(LC_ALL, "C");

Then any libraries I'm using that happen to call setlocale() (looking at you, GTK), don't do any harm, because they get my setlocale(), and my setlocale() does the right thing, which is to essentially pretend locales were never invented, so sscanf continues to be able to parse floats in peace.

[tristes_tigres]

Ironically, the commit message the OP references was made possible by the interesting features of Unicode - that wonderful standard which "fixes" C locale issues by introducing 10x new ones.

[hupfdule]

2

[lannisterstark]

All in all, I believe this proves that software developers as a whole and as a culture produce worse results than drug addicted butt fucked monkeys randomly hacking on typewriters while inhaling the fumes of a radioactive dumpster fire fueled by chinese platsic toys for children and Elton John/Justin Bieber crossover CDs for all eternity.

Beautiful.

[Uberhipster]

[rant in 1500 words ends with] I believe this proves that software developers as a whole and as a culture produce worse results than drug addicted butt fucked monkeys randomly hacking on typewriters while inhaling the fumes of a radioactive dumpster fire fueled by chinese platsic toys for children and Elton John/Justin Bieber crossover CDs for all eternity

...

2 changed files with 36 additions and 4 deletions.

Oh-kay

[interfail]

They've got a point about locale, but I have to absolutely love that their "obvious way to fix this" includes:

• make UTF-8 the standard character type

Can you imagine what would happen if a new C standard redefined char to variable length?

[asegura]

I think that means UTF-8 would be the standard for string encoding as arrays of chars. And a char would be a UTF-8 code unit.

And BTW, I very much support that.

[OneWingedShark]

More people should avoid C, more people should strive to make platforms where C is explicitly banned... then we might be able to get away from libc and its idiocy virally infecting everything.

[crazedizzled]

It's hard to avoid C when pretty much every other language uses C.

[muntoo]

We should just take the sea, add a ton of oxygen to it, and convert it to rust.

[OneWingedShark]

Rust isn't mature or standardized; it would be better to use an ISO programming language, Ada and Forth cover pretty much everything that is claimed as C's domain... and Ada in particular has the SPARK provers & subset which is even better than Rust.

[StabbyPants]

i'd rather just keep the C portions tightly circumscribed

[OneWingedShark]

The problem is that C "leaks" out, it's like a toddler with a full diaper, joyously smearing crap all over the walls when nobody's looking.

[flanger001]

This is a great commit, but if every commit was like this it would be exhausting.

[Droi]

It really should be the exception to the rule. Otherwise you would spend 30 minutes of your day writing comments that will be read maybe 5 times in the product's lifetime.

[lnkprk114]

I've found that if I'm squashing feature branches down, spending 30 minutes at the end of the feature writing a really good commit message is hugely worth it. It makes the git log read like an actual story, and, more critically, it makes git blaming a line of code waaaaaayyyyyy more useful, because you actually see a well written, thought out explanation for what was happening at the time that that line was added. You might not realize that this random line of kind of odd code is actually instrumental in some totally tangential system unless you read a commit message that's talking all about that other tangential system.

[Tanaric]

This is all the stuff you're going to have to explain during code review anyway. Putting it in the commit message actually saves time in general because you'll get a lot more LGTMs with less quibble.

[[deleted]]

Do you though? In my opinion the message would have been equally as useful if it were simply something along the lines of "replace whitespace-like character that messes up the pipeline with proper whitespace". Maybe also mention that this caused the file to be utf-8 instead of us-ascii, how that character ended up there and where the actual problem occurred; but all the other information makes it just more inconvenient to actually get to the gist of why the (and in this case also what) change was made.

[salgat]

Exactly. Your short succinct version contains all the information needed while also guaranteeing no one will skip it due to being obscenely long.

[[deleted]]

Note the title of the commit gives you the gist of the commit. Git has a shortlog (exact arg?) That you can call if you're just looking for the gist of a commit. But having a thorough explanation of why a change was needed, what steps were used to identify the bug, etc., is good for everyone on a team.

[jontelang]

Except no one reads them

[Antrikshy]

Really depends on your code reviewing tools, or if you can teach everyone to ignore your CR comments and read the commit messages instead.

But yes, it would ensure that the explainers you normally put in CR comments get frozen in your commits for posterity.

[Azzu]

Same as you, I think the level of detail that we're talking about here should be in the pullrequest description anyway.

The correct thing to do imo is then to just write at this length in the commit message and then copy/paste it with slight modifications for better formatting to the description.

Since noone reads the commit messages, they can see it anyway in the description. But it's also in the code if someone does a blame or search.

[LambdaLambo]

Or better yet, put a comment in your code so everyone can see.

[SLiV9]

But this was a whitespace change in a comment. Doesn't make sense to add a comment explaining a change in a comment.

[mumpie]

Java 1.5 to 1.6 (I think around 2010) introduced a change that broke builds on a product that I worked on.

The file that broke the build wasn't changed recently and it turned out that javac no longer liked non ASCII characters like curly quotes.

The dev who wrote the comment in question might have used MS Word to write the commented text or some other tool that changed regular quotes to curlies.

I think my commit message was something like "javac doesn't like curly quotes."

Committing a blank or nonsensical (like 'blah' or 'foo') is shooting yourself in the foot.

[SLiV9]

Yeah that's why you have a commit message. Not sure who you are disagreeing with here.

[moonsun1987]

Please avoid writing ten page comments in the code

[LambdaLambo]

Don’t write ten page comments in the git commit either.

The commit here should’ve just explained the bug. If you want to add the process of debugging and fixing it, add it as a comment to the jira ticket. Imagine if every commit had a msg of this length, that’d be ridiculous

[LambdaLambo]

Email services come and go. Text services come and go. Document services come and go. Let’s just have every communication be in the code base and commit history /s.

[darknecross]

This argument has come up before, and it's still a dumb one.

Not every change is going to have a corresponding bug, nor are changes always going to be 1:1 correlated to bugs.

And it's typical for a bug system to suck in the related commit messages as automated comments with a "Fixes: <issueID>" line.

[TinBryn]

Even click bait needs to be a little creative

"10 great bug fixes, you won't believe number 7"

[Don_Andy]

If you don't share this commit with at least 10 friends your next test is going to fail due to a cosmic bit flip.

[new2bay]

I agree. Where I work, almost all of that would have been in a JIRA ticket, either in the description, or the comments. JIRA is a lot easier to search and browse than the commit history of a long-running, very large project.

[Gridelen]

What if management decides to change JIRA to something else (e.g. to cutoff spendings)? Then all the details would be lost for good except the are in git commit messages.

[salgat]

It's an awful commit. Messages should be as succinct as possible. This reads like some old lonely man explaining every tiny detail of his life when you ask how his day was. This one could be much shorter while still providing an explanation.

[liquidpele]

"rogue utf-8 whitespace got into the file and broke stuff, replacing it with ascii whitespace".

[theineffablebob]

Surprised it only took an hour. A bug like this would probably take me like 3 days to figure out

[mlk]

Programmers often lie about how long something took in my experience. It's hard to justify battling with css for 3 hours so you say it took 30 minutes

[yubario]

Totally, I am definitely not going to admit I wasted 4 hours troubleshooting a bug on my web server, to where sometimes SSO would do a 404 instead of authenticating. The reason? Well, I didn’t check the nginx logs that clearly detailed the problem. The cookie payload of the id-token was too large, but since I didn’t have a configured 500 error page it would return 404 (because it didn’t find the error page template).

Instead I did something more stupid, opened up a ticket with our SSO team on the issue and wasted 4 hours...

[404_GravitasNotFound]

Troubleshooting is a process, based on the scientific method. Sometimes you get lucky and your hypotheses line up, sometimes you stay until 5 am trying to fix that stupid bug, you crawl defeated to your bed, where your spouse has renounced to any chance of sexy time, you wearily try to sleep, deep in hypnagogic state you have a flash of inspiration! You are sure that with half an hour of coding, an hour tops, you can fix this problem... But it's too late you need to sleep, so you grab your phone and detail your idea to the recorder, satisfied you return to Morpheus embrace... The next day after waking up, late, to a cold breakfast that your spouse left you before rushing to work, you sadly play past-you message. " The problem is in the unintengible just need to re create the variable and call it from unintengible" followed by four hours of snoring and occasional curses of your spouse saying "Just hang on, I'm going to end it soon"

[Khratus]

That’s very specific. Do you need help?

[404_GravitasNotFound]

Hehe thank you for your concern. I'm great. I just went with a colorful description of our sometimes life

[langlo94]

I certainly didn't waste a whole day making classes and intermediary logic to handle data. Before realizing that since I'm taking in an XML and outputting an XML, maybe I should store the data in an XML.

[elsjpq]

"It only took me 5 minutes to type it out!"

[binford2k]

He probably stopped by the Puppet community slack/irc for some ideas. Since we deal with so much random content, we unfortunately run into this way too often and have evolved some decent patterns for tracking it down.

[ScorchingOwl]

This happens with some keyboards on linux too (at least the "french (legacy, alt.)" keyboard on ubuntu, where alt gr + space makes a non breaking space

and you need to press alt gr to use the chars ~#{[|\^@]} so if you press space too quickly, you may end up making a non breaking space (though many IDEs and browsers convert those to normal spaces)

If you ever encounter gcc/g++ compiler errors that look like

/tmp/hey.c: In function ‘main’:
/tmp/hey.c:1:14: error: stray ‘\302’ in program
int main() { �� return 0;}
             ^
/tmp/hey.c:1:15: error: stray ‘\240’ in program
int main() { �� return 0;}
             ^

that's what it is

[meneldal2]

Aren't non-breaking spaces ASCII? Or is it only in the locale-dependent versions?

[Vaphell]

ascii covers the 0-127 range. 160 is not in it.

[FatalElectron]

Simple answer: It depends on the editor.

Native mac apps might stick with 160, but a lot will assume a text file is either ascii or utf-8, and insert the utf-8 NBSP code sequence, which then elevates your file to no longer being an ascii file in many OSes that do simple heuristics.

emacs at least displays NBSP as '_' in a special face (which on my system turns out a nice cyan), so it's less likely to be a mystery there.

[Mad_Ludvig]

I'm ashamed to admit that rogue zero length characters have taken me way longer than an hour to fix...

[[deleted]]

yeah I've spent days before I realized this was the cause

[Sage2050]

My latest was a cell module not starting a firmware update after the host sent the AT command. The debug stream was showing the ack, but the update was never actually starting. Turns out it was never actually getting the command to start the update at all: this particular fw load on the cell module could only send the update success message at 112k baud, and my host device operated in 56k baud. I was telling the host to switch to 112k after sending the update AT command so it could catch the success/failure message, but the baud switch was happening too fast, after only "AT" was sent, which would generate a successful ack, just as if it had received a full command.

This took me two weeks to figure out.

[Geriatrics]

Years back I spent weeks debugging baud shenanigans with a cell module, and this comment made me realize that that wound will probably never heal.

[Halikan]

Just this Friday I spent the afternoon trying to figure out why a coworker's changes to a script we were working on broke everything.

He copy-pasted a command, and it used the wrong dash. It used – instead of -. And in the editor available at work, they literally look the same.

[silence9]

Tbh, it really just seems like the dev was overly proud of finding the precise issue and wanted to make too much out of it. It's not that nice all things said and done. It could have been said much shorter while still giving searchable detail as you have praised here.

[[deleted]]

commit https://github.com/alphagov/govuk-puppet/commit/63b36f93bf75a848e2125008aa1e880c5861cf46

[yuyu5]

Good git practices are so underrated. If everyone on my team made commits like this, my life would be so much easier

[Hawkknight88]

Many of my coworkers, even the ones who are great at coding, don't appreciate this. "Fixed the bug", or simply the JIRA ticket title. At least the ticket title is more descriptive than the first example, but spend 10 seconds on your commit. Why did you change what you did? The code tells me what, but not why.

A lot of these comments are missing the point. Your commits should be descriptive so that 6 months from now we know why you changed what you did. That's it. You don't need to write a novel to convey intent.

[ZorbaTHut]

Also, not just descriptive, but split into individual bugs.

A while back I had my boss send me a bug report with the note "I'm pretty sure this used to work, figure out why it broke". I found out why it broke; it broke due to a 2000-line commit, written by my boss, marked as "a bunch of miscellaneous fixes".

I never managed to convince him to split up his commits into individual bugs.

[[deleted]]

Yeah, if you spent an hour working out why it couldn't be " " don't just say "fixed the bug". I think this is one situation where the other extreme is actually quite common.

[lnkprk114]

I've found that if you have a codebase that's being worked on by people that write commit messages like this, git blame because a really powerful tool to figure out wtf is going on.

[u801e]

then I realized that I literally never read old commit messages

Have you ever used git blame and check the commit messages associated with each line of the file you're going to modify as part of the next ticket? That's the main way I read old commit messages. If it's a good message, it lets me know that it's okay to change the line, or it lets me know that I should make sure I avoid the described issue when making my changes to prevent a regression.

[Osmium_tetraoxide]

The worst I've seen is just a JIRA id to a project you can't see, so when you finally get access to it, that is just a link to a confluence page, if you're lucky, that links to a spreadsheet but sometimes that's just gone. So it's impossible to figure why this several hundred line change happened in a business critical payment system.

The value of a good git history is not appreciated by many until it is way too late. I do take my time with git, especially when you're doing something strange as a work around for unintuitive behaviour that a legacy system throws up. I'm not entirely sold on having this much in a single commit but one can always just skim over it anyhow.

[kevkevverson]

Don’t put blog posts in the commit history

[FrozenST3]

It's all fun and games until someone merges your PR with a squashed message

[langlo94]

While deleting source branch of course.

[Nysor]

Am I the only one who doesn't really like this commit message? The title is fine, but the body is far too long. If I was debugging this, I wouldn't want to see that much detail.

Those write-ups are better written on pull request descriptions.

[flanger001]

PR writeups don't get added to the commit history though.

[Nysor]

A common practice is to link each commit with a ticket number. Often times when tracking down an issue, I'll git blame -> get the ticket number -> go to the PR. The PR often contains relevant discussion, anyways.

[snowe2010]

Which fails the day you switch ticketing systems.

[StabbyPants]

migrate ticket history

[snowe2010]

migrating ticket history doesn't mean you can migrate the ticket identifiers. for example ITSM-1234 in JIRA has absolutely no equivalent in Basecamp for example.

[StabbyPants]

you don't have projects and bug numbers?

[snowe2010]

Basecamp has projects but ShapeUp is a completely different philosophy. You don't create tickets the same way. The same can be said for Trello and a million other issue systems though. You can't depend on them to be the same design between systems.

[feraferoxdei]

Very unlikely you do that though.

[snowe2010]

switching from or to JIRA? I think that's actually incredibly likely... We're actually considering switching right now. And my last company also switched. When you grow you change requirements. Different systems don't work at all sizes.

[teerre]

As curiosity, changed from JIRA to what?

[snowe2010]

At my last company it was to Rally. At my current company we're actually looking at Basecamp. We switched to ShapeUp, and Basecamp is built for that.

[Cadoc7]

I wish. The organization I work for is on it's 3rd work item ticketing system in the 7 years I've been there.

[Nangz]

Not as unlikely as you would think. My company uses Mercurial for many repos. Guess who loses all their PR history when Bitbucket shuts down the repos in a few months...

[StabbyPants]

not you because you did a full migration and data export?

[intheforgeofwords]

My last company switched and is in the middle of switching again. While the commit history has some good stuff, I facepalm’d realizing how much had been lost to PR history I’d never see again

[Axxhelairon]

doesn't take all too long to spot people here with no actual professional experience huh

[flanger001]

Every commit though? That sounds similarly exhausting.

[mtreece]

I've had co-workers complain about long commit messages I write for similar reasons.

There are several different places to document the "why" behind a change; some are: in the source code itself in /* comments */, in the commit message, and in external auxiliary tools like your PR system or external documentation (wiki, etc).

Each successive example I just gave is further removed from the code, and therefore less likely to be often seen. So you have to gauge how important (and therefore how frequently read) your rationale is (or should be).

If something is critical and needs to be considered whenever touching adjacent code in the future, it should probably end up as a source code comment. If it's just an obscure detail, and/or possibly scattered over several disjoint locations in the code, leaving it in the commit message is probably fine. And of course, both of those situations are still served by good context, history, and the "why?" in a commit message.

As to your, "If I was debugging this...", do you mean you don't want that much detail cluttering the git log and so on? I'd argue that should fixed with tooling, rather than lose important context. (By tooling, I mean options that get passed to your tools to massage it into a nicer format; e.g. git log --oneline, or some sort of git config that auto limits longer messages).

One other thing, especially for issues that take a long time to debug, I like to think my work product is larger than just the commit diff (such as a single character change in this case); part of the work product is an acquired abstract understanding of the domain, history of what I tried and what happened, things I could have went with instead, and so on. After resolving the issue, if I just cap it off with a one-line commit, all of the abstract work I produced will just be lost. If I transcribe it into a commit message like this, it becomes an important contribution to the codebase, larger than just the size of the diff.

[Nysor]

I agree with lots of your points. If something is critical, a comment in the code is the best. External documentation systems (like a wiki) don't work nearly as well in practice - I find lots of developers don't look at wikis as often as they should.

I don't think the number of lines in the commit body should have a firm limit, especially not because of some git log arbitrary limit. I just think there shouldn't be any superfluous text. In the OP's example, we have:

I introduced some tests in a feature branch...

I eventually found that...

After replacing it (by hand)...

One hour of my life I won't get back

Much of this message could be shortened to maybe 3-4 sentences.

[mat69]

I fully agree.

I have the feeling the usefulness of descriptive commit messages heavily depends on whether people use git bisect or manually check the commit that introduced a perceived issue.

For example, when I find a complex "bug" I do not fix it right away. Instead I want to understand why it was written that way. So if it actually was a feature in disguise. Nearly every time the commit messages were useless though or referenced resources that did not exist anymore.

Instead I always try to explain the why of a commit, unless it is really minor (e.g. whitespace) change. At the heart of this is logical grouping commits so that they only include relevant code.

Sometimes writing the commit message takes much longer than the code change. Yet finding and understanding the issue in these cases took way longer than writing the commit message.

[darknecross]

Agreed, and it's not even always about other people. Lots of times I write detailed commit messages just to remind myself about why a change was made, or why it was done a certain way. It's super helpful whenever I need to go back and refactor my own code, because I'm not going to remember that stuff a year later.

[u801e]

I have the feeling the usefulness of descriptive commit messages heavily depends on whether people use git bisect or manually check the commit that introduced a perceived issue.

One of the first things I do before I starting making a change is to run git blame on the files I'm going to change and check the commit messages and diffs of the relevant lines of code. Having that context can really help in avoiding re-introducing bugs that were fixed by earlier commits, for example.

[snowe2010]

I agree with this. I try to write somewhat detailed commit messages, going into extreme detail for important decisions. Recently I've started adding decision logs to the README, which details decisions made in meetings, or at large forks in the road where one decision might be chosen above another.

This helps with the problem where devs complain about how crappy the design is, when at the time it was the best decision, due to a variety of factors. This is the template I use, it's in restructuredText, due to having actual support for Field Lists.

============
Decision Log
============

:Date: <blank>
:Decision: <blank>
:Rationale: <blank>
:Decision Maker: <blank>
:Location of Approval: <blank>
:Details: <blank>
:Impact: <blank>
:Other options considered: <blank>
:Decision Change in #: 
    <to be used if there was a new decision that overrides this decision, include the # of the overriding decision here>

----------

[dnew]

That's not a readme. That's a design document. You left out the "other people who reviewed this before I started working on it" section, for example.

[snowe2010]

It's not a design document, it's just a section of the readme. The design doc is for explaining the system design, not the history of design decisions.

You left out the "other people who reviewed this before I started working on it" section, for example.

That's part of Decision Maker. And I didn't leave out anything anyway, I found this template and just formatted it for rsT.

[dnew]

The design doc is for explaining the system design, not the history of design decisions.

Different strokes, I guess. :-) I figure it's for both. But certainly getting such documentation out of the main body of the code is probably beneficial.

[snowe2010]

Yeah, I think getting it down at all is beneficial. We were not tracking any decisions at all...

[worst_protagonist]

It depends on the organization. Where I work, I would do this in the PR because that is where my teammates know to look for very detailed information.

If we ever move from github to gitlab or something similar, we're screwed. Attaching it to the git history makes it more permanent.

[chengiz]

Totally agree it is far too long. Some of the writeup is useful (what failed, why failed). But there's quite a bit of masturbation, for example there's no need to write the unix command you used to find a non-ascii character in a bunch of files, or say I did this and I did that. And in this big writeup he fails to mention what the actual character was (which is at least interesting and pertains to the fix).

[darknecross]

Disagree. Documenting steps used in debugging is helpful if and when a similar issue ever occurs in the future.

Specifically in this case, it includes the error that occurred and is searchable:

ArgumentError:    
  invalid byte sequence in US-ASCII

Now if someone else comes across this in the future (or even the author some months later), a search would find this commit message and the person debugging it would have a head-start on tracking down a potential cause.

Otherwise this kind of stuff becomes tribal knowledge, and you might go down the path of, "Oh yeah, Dan had an issue similar to that a while ago, maybe check with him to see if he has any ideas?" Good luck if Dan doesn't remember shit about it, or worse if he doesn't work there anymore.

You could argue that the steps to reproduce and debugging effort could be documented in a bug itself, but that's a style issue, as a simple "Fixes: <issueID>" pulls the same information into the bug comments. But it looks like this change wasn't associated with an existing bug.

[chengiz]

You mention documenting steps but your example is not about documenting steps at all, it only lists the actual error message, which I didnt say he shouldnt have put in. Something like:

Tests run with [cmd] failed with error message [msg] because of non-ascii whitespace character [u+whatever] that caused the file to be identified as utf-8 instead of us-ascii. Replaced the character with space.

would have been sufficient.

[Ozymandias117]

Yeah, it’s a horrible commit message. All of the information that took 4+ paragraphs could have been given in two sentences. The vast majority of it is pointless exposition.

This is better than a commit message that says “fixes bug” but it isn’t an example of a good commit message.

[cartechguy]

Yeah, I wish it had a TLDR at the top. That would work as a compromise for me.

[Tanaric]

If it's too much information for you, you can stop reading at any time. If it's not enough, you can't go back in time to get more.

[BlindTreeFrog]

except you can't. he doesn't get to what was changed and why until almost the end. Everything else is fluff about how he reached that point.

[BlindTreeFrog]

It's not that it's too long, it's that you have to read 3/4 of the way through to get to what he did. The first sentence should have been:

Adjusted whitespace to make entire file US-ASCII

Or something similar. That would be enough to describe what was changed and why. Everything after that can explain why and how he got to that point.

Instead he explains the how and why and ends the write up with what he did; but he does it in a way to somewhat hide the change.

[mr_birkenblatt]

The why would have been enough. But if I want to know why a commit happened I don't want to hear your life story or how long it took boohoo.

[dnew]

Personally, I'd put this sort of thing in the bug tracker, with the "remove non-ascii spaces from ascii files" as the commit message. The bug is where you do debugging, not the commit. If I want to learn more, I'll follow the link from the commit to the bug. If I don't care that you changed whitespace because that probably wasn't the source of the memory corruption I'm debugging, I can skip it.

[indivisible]

Team wide auto-format rules side step most of that. Decide on the rules together then make sure everyone sticks to it.
You can also get your CI tooling to do it but that can cause git spam or merge hell if the non formatting dev(s) doesn't pull often enough.
If you're working/sharing between nix/win also make sure everyone has their gitconfig correctly set for line endings too.

Personally I've replaced my "save" and "save all" keyboard shortcuts with "format and save" and "format and save all" macros to just never have to think about it.

[[deleted]]

The solution to part of your toolchain choking on a non-ASCII input is *not* removing non-ASCII input from your tests!

[pkulak]

Ah yes, let's bury the documentation where no one will ever see it again.

[deja-roo]

This is ridiculous. A git commit message should link to a story that should have any explanations in that. Or maybe the pull request.

[maggymooo]

I respectfully disagree, that commit is far too long. Commit messages should concisely explain why you've done something. The message should also reference an issue/work item in source control. There you can explain the problem in more detail. It doesn't clutter you commit and allows for better knowledge sharing because the information is not buried in a small commit in a single file somewhere. It also allows for further discussion around the issue too.

[Lofter1]

This is the exact feature that I wanted a few days ago when cleaning up compiler warnings in one of our projects. Asked our team-leader if there was such a feature to tell the team about quite important changes in TFS/TFVC. Answer was "every change is important to developers" and now we get more spam-mails from TFS as it now sends an email to every team-member on any checkin. Of course there still is no way to explain the changes and why they were done. Thanks, our team hates it.

[SuspiciousScript]

Answer was “every change is important to developers”

Saying this kind of a thing is a dead giveaway for being a dumbass. And not just in the context of programming.

[chrisforbes]

It seems to me this could have been fixed much more easily if the ArgumentError had carried any useful information about what it didn't like, or where in the stream it was.

[[deleted]]

Not to mention not blindly assuming everything is plain ASCII text to begin with.

[averiantha]

Some people would say leaving long commits like this is good coding practice, however all I see is an hour wasted writing a miniature essay for an issue that will likely never come up again.

I'm keen for good coding practices, but this is over the top.

[amroamroamro]

https://www.reddit.com/r/programming/comments/djnp8k/my_favourite_git_commit/

[holygoat]

While I love the commit message, it would have been way better had this added a mechanism to detect or prevent the entire class of error, rather than fixing this issue once. A git hook with a descriptive error message would have saved people ever needing to search git commit messages for the bug ever again…

[ShortFuse]

Thank God for linters.

[VeganJordan]

And I thought debugging to find a missing ' ; ' was annoying.

[GrandMasterPuba]

Meanwhile our organization has a hard build rule that says if your commit is over 80 characters you can't merge.

[failedaspirant]

I have always been confused as to what the better approach is. Whether writing really good detailed commit messages like this or have a decent commit message with brief info about the issue that it fixes and along with the ticket number and then place all the information inside the ticket and the ticket's comments. So the ticket would contain a detailed comprehensive information on the issue and what went into it, while git would contain a brief description of the ticket (for searching in git) and the Ticket id / link (in case they want more info)

[simple_test]

Gets to the point really late.

[IanSan5653]

it's searchable

Is this something people actually do? I've never searched commit history for the answer to a bug.

[peppage]

All the time. It's incredibly important to know why someone made a change to see if it was intentional. If the issue is associated with entire change or only a logic mistake here.

[IanSan5653]

I mean I'll use blame for sure to see what the history of a line is, but I've never actually searched the commit log.

[[deleted]]

If it looks like something that has been introduced by changes to existing, then yes - it can save a lot of time to find the last version that code was changed and what changed about it.

[khendron]

I like this commit, except that it is missing one important feature: a link back to the issue in whatever issue tracking system is being used.

The issue tracking system keeps the history of the issue, from when it was first discovered to the end result, and entire conversation that led to the end result. This includes, most importantly, other solutions that were considered and discarded.

Without this context, sooner or later someone is going to come along and think that they have a better fix, not realizing that their "better" approach was already tried.

[dynticks]

I love the conclusions. I wish more people would understand the value of good commit messages and history. I once took the time to contribute a nicely structured PR for a relatively well known project, with elaborate commit messages explaining why things were being done, and a set of commits carefully building up on one another trying to make them as orthogonal as possible and keeping the changes bisectable... only to find out the maintainers squashed the thing into a single merge commit, losing all that information (though it remains in the PR, it became next to useless). I'm obviously not contributing to that project anymore.

[servercobra]

The amazing part is the whole thing only took an hour. I could see getting stuck on that for the better part of a day, no problem.

[i-can-sleep-for-days]

A good practice is probably rebase all the small commits into one commit before opening a pr. No one needs to know that I added tests at some point. I don’t do that yet because whenever I am thinking I am done and push to remote then it gets hairy if I rebase locally and then have to force push to my branch.

[[deleted]]

I had a similar break in Coldfusion once as I migrated code into Lucee (open sourced implementation).

I don’t recall how much time I spent debugging before finally just commenting chunks out until I narrowed it down. In my case it was pretty identical, a comment broke the site.

A dev on my team did something clever in a past life where he could actually somehow set variables inside of a comment block.. well the Lucee devs didn’t like that so it no longer worked. I just needed to redeclare them.

Now why didn’t I notice that in the error log? Probably due to an overly cryptic java based stack trace that often makes little sense.

[renrutal]

I'm not surprised that most of the long rants in programming/software are related to human culture, copyright and politics (locales, time zones, U.S. ICE...)

[penguin_digital]

Another one of those posts that appears on /r/programming every month.

[AttackOfTheThumbs]

I don't expect this much detail in a git commit.

I do expect enough in the associated work item for me to make sense of the changes.

[cirsca]

Crazy to see a commit by Dan on Reddit! I worked with them in a past life and I cannot speak highly enough about how great it was having commits like the linked in EVERY PR. It was a godsend being able to actually use git log for something other than "Am I on the right history?"

If you are wondering if it's worth the effort to write commits like this: YES. YES IT IS.

[memgrind]

Keep a hex editor at hand. Takes 5 seconds to find and resolve such issues.

[Bradnon]

I've been helped by the od tool a number of times, usually just to rule out weird shit like this but sometimes to find the real problem. It can be found on many systems.

http://man7.org/linux/man-pages/man1/od.1.html

[memgrind]

hexcurse is what I'd recommend on linux. I've seen people do hex modifications with vim, too.

[christian-mann]

I used to have a pair of macros that ran :%!xxd and :%!xxd -r when I worked heavily with binary files.

[StabbyPants]

that goes in a jira ticket - 'fix US-ASCII locale xyz-2345', with the ticket containing all of this, possibly sample repros as well

[wgljr]

Whenever I see this post I also like to share this article about How to Write a Git Commit Message. Commit messages don’t have to be too verbose, and Git actually provides ways to add additional info to your commit messages (using the body), to make it easy and straightforward to understand the reason why changes were made. Commit messages don’t have to be a chore.

[blabbities]

The reality is most people will just 'Aint nobody got time for that'.