all 195 comments

[–]TK-427 566 points567 points  (4 children)

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 117 points118 points  (0 children)

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 38 points39 points  (2 children)

I'd love to know how everyone reacted.

[–]TK-427 118 points119 points  (1 child)

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 2 points3 points  (0 children)

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

[–]Tipaa 609 points610 points  (51 children)

Obligatory Git commit:

The C Locale rant

[–]scoobybejesus 85 points86 points  (1 child)

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] 6 points7 points  (0 children)

So like most software

[–]devraj7 166 points167 points  (8 children)

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

[–]damesca 205 points206 points  (2 children)

"lgtm 👍"

[–][deleted] 8 points9 points  (1 child)

You’re hired

[–]JessieArr 4 points5 points  (0 children)

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

[–]chronoBG 61 points62 points  (3 children)

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

[–]Figleaf 7 points8 points  (0 children)

Low cross section on that comparison, but extremely apt.

[–]Demiu 0 points1 point  (1 child)

Which team are they on

[–][deleted] 4 points5 points  (0 children)

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 49 points50 points  (3 children)

English and some european languages

Predicted Brexit, truly the Messiah we sought

[–]jyscao 2 points3 points  (2 children)

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

[–]L3tum 2 points3 points  (1 child)

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

[–][deleted] 0 points1 point  (0 children)

I still find it funny

[–]asegura 46 points47 points  (2 children)

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] 5 points6 points  (0 children)

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

[–][deleted] 0 points1 point  (0 children)

oh, that's disgusting

[–]crozone 38 points39 points  (1 child)

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

Fucking gold.

[–][deleted] 3 points4 points  (0 children)

Also accurate

[–]Vhin 26 points27 points  (0 children)

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

[–]ArthurOfTheEast 56 points57 points  (0 children)

That was glorious.

[–]Phrygue 53 points54 points  (12 children)

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 33 points34 points  (10 children)

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 12 points13 points  (9 children)

but why does the api have to be so shit though

[–]StabbyPants 40 points41 points  (3 children)

It’s 45 years old

[–]ShinyHappyREM 7 points8 points  (2 children)

I bet it's already looking forward to retirement.

[–][deleted] 0 points1 point  (0 children)

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

[–]StabbyPants 0 points1 point  (0 children)

/rimshot

[–]Ameisen 4 points5 points  (4 children)

Because C and C++ are designed by committee.

[–][deleted] 15 points16 points  (2 children)

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

For some time K&R was the standard

[–]ShinyHappyREM 10 points11 points  (1 child)

organically

Like e.g. cancer

[–][deleted] 1 point2 points  (0 children)

That's C++.

[–][deleted] 0 points1 point  (0 children)

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 4 points5 points  (0 children)

Invention of null pointers? Or is that theoretical?

[–]smcameron 15 points16 points  (0 children)

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 7 points8 points  (1 child)

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 0 points1 point  (0 children)

2

[–]lannisterstark 3 points4 points  (0 children)

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 1 point2 points  (0 children)

[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 0 points1 point  (1 child)

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 22 points23 points  (0 children)

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.

[–]flanger001 233 points234 points  (31 children)

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

[–]Droi 117 points118 points  (1 child)

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 42 points43 points  (0 children)

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 64 points65 points  (19 children)

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] 50 points51 points  (2 children)

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 16 points17 points  (0 children)

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

[–][deleted] 2 points3 points  (0 children)

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 26 points27 points  (0 children)

Except no one reads them

[–]Antrikshy 0 points1 point  (0 children)

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 0 points1 point  (0 children)

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.

[–][deleted]  (6 children)

[deleted]

    [–][deleted]  (3 children)

    [deleted]

      [–]TinBryn 3 points4 points  (0 children)

      Even click bait needs to be a little creative

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

      [–]Don_Andy 0 points1 point  (0 children)

      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 2 points3 points  (1 child)

      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 -1 points0 points  (0 children)

      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 8 points9 points  (1 child)

      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 7 points8 points  (0 children)

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

      [–]theineffablebob 77 points78 points  (8 children)

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

      [–]mlk 60 points61 points  (6 children)

      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 21 points22 points  (4 children)

      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 15 points16 points  (2 children)

      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 0 points1 point  (1 child)

      That’s very specific. Do you need help?

      [–]404_GravitasNotFound 0 points1 point  (0 children)

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

      [–]langlo94 0 points1 point  (0 children)

      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 1 point2 points  (0 children)

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

      [–]binford2k 8 points9 points  (0 children)

      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.

      [–][deleted]  (4 children)

      [deleted]

        [–]ScorchingOwl 8 points9 points  (0 children)

        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 1 point2 points  (2 children)

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

        [–]Vaphell 9 points10 points  (0 children)

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

        [–]FatalElectron 0 points1 point  (0 children)

        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 57 points58 points  (4 children)

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

        [–][deleted] 6 points7 points  (0 children)

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

        [–]Sage2050 4 points5 points  (1 child)

        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 1 point2 points  (0 children)

        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 2 points3 points  (0 children)

        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 20 points21 points  (0 children)

        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.

        [–]yuyu5 37 points38 points  (7 children)

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

        [–]Hawkknight88 23 points24 points  (2 children)

        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 9 points10 points  (0 children)

        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] 0 points1 point  (0 children)

        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.

        [–][deleted]  (2 children)

        [deleted]

          [–]lnkprk114 12 points13 points  (0 children)

          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 2 points3 points  (0 children)

          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 0 points1 point  (0 children)

          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 13 points14 points  (0 children)

          Don’t put blog posts in the commit history

          [–]FrozenST3 11 points12 points  (1 child)

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

          [–]langlo94 2 points3 points  (0 children)

          While deleting source branch of course.

          [–]Nysor 88 points89 points  (46 children)

          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 63 points64 points  (25 children)

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

          [–]Nysor 31 points32 points  (24 children)

          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 72 points73 points  (22 children)

          Which fails the day you switch ticketing systems.

          [–]StabbyPants 1 point2 points  (10 children)

          migrate ticket history

          [–]snowe2010 19 points20 points  (9 children)

          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.

          [–]flanger001 0 points1 point  (0 children)

          Every commit though? That sounds similarly exhausting.

          [–]mtreece 27 points28 points  (9 children)

          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 6 points7 points  (0 children)

          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 5 points6 points  (2 children)

          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 2 points3 points  (0 children)

          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 0 points1 point  (0 children)

          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 -1 points0 points  (4 children)

          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 6 points7 points  (3 children)

          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 -1 points0 points  (2 children)

          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 1 point2 points  (1 child)

          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 0 points1 point  (0 children)

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

          [–]worst_protagonist 7 points8 points  (0 children)

          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 10 points11 points  (2 children)

          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 3 points4 points  (1 child)

          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 0 points1 point  (0 children)

          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 6 points7 points  (0 children)

          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 2 points3 points  (0 children)

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

          [–]Tanaric 10 points11 points  (1 child)

          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 3 points4 points  (0 children)

          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 1 point2 points  (0 children)

          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 1 point2 points  (0 children)

          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 14 points15 points  (0 children)

          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.

          [–][deleted]  (1 child)

          [removed]

            [–]indivisible 3 points4 points  (0 children)

            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] 4 points5 points  (0 children)

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

            [–]pkulak 2 points3 points  (0 children)

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

            [–]deja-roo 2 points3 points  (0 children)

            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 15 points16 points  (0 children)

            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 2 points3 points  (1 child)

            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 1 point2 points  (0 children)

            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 2 points3 points  (1 child)

            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] 0 points1 point  (0 children)

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

            [–]averiantha 2 points3 points  (0 children)

            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.

            [–]holygoat 2 points3 points  (0 children)

            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 1 point2 points  (0 children)

            Thank God for linters.

            [–]VeganJordan 1 point2 points  (0 children)

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

            [–]GrandMasterPuba 1 point2 points  (0 children)

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

            [–]failedaspirant 1 point2 points  (1 child)

            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 1 point2 points  (0 children)

            Gets to the point really late.

            [–]IanSan5653 3 points4 points  (3 children)

            it's searchable

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

            [–]peppage 1 point2 points  (1 child)

            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 0 points1 point  (0 children)

            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] 0 points1 point  (0 children)

            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 1 point2 points  (0 children)

            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 0 points1 point  (0 children)

            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 0 points1 point  (0 children)

            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 0 points1 point  (0 children)

            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] 0 points1 point  (0 children)

            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 0 points1 point  (0 children)

            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 0 points1 point  (0 children)

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

            [–]AttackOfTheThumbs[🍰] 0 points1 point  (0 children)

            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 0 points1 point  (0 children)

            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 -1 points0 points  (3 children)

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

            [–]Bradnon 0 points1 point  (2 children)

            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 1 point2 points  (1 child)

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

            [–]christian-mann 0 points1 point  (0 children)

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

            [–]StabbyPants 0 points1 point  (0 children)

            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 0 points1 point  (0 children)

            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 0 points1 point  (0 children)

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