Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

My Favourite Git Commit

fatbusinessman.com

Post reply on HN
Print view View original

81–90 of 389 posts

Re: My Favourite Git Commit

#81

Post by Symbiote »

hleszek wrote: post #10

To avoid those kind of issues, non-ascii characters are forbidden in our code base. They are automatically verified in a pre-commit git hook.

I intentionally add non-ASCII characters to our code, so that an incorrectly configured IDE or bad tool fails.

75% of the development team has at least one non-ASCII character in their name, so it would be pretty rude otherwise.

It's much better to knowingly reject a tool at the start, since it can't handle ordinary characters, than find out a year later with the first e.g. British customer that it can't handle "£".

Re: My Favourite Git Commit

#82

Post by dijksterhuis »

tigershark wrote: post #63
drej wrote: post #41

I love these commits. Then don't have to be this verbose, but they have to tell a story of why things were done. I can sort of deduce the what from the code itself, but the why is sometimes shrouded in mystery. I started with these explanatory git commits a few months ago and they are super useful, even if you're just reading your own commits from some time ago.

To me they are red flags because it means that very likely who writes this kind of commit messages doesn’t use a proper issue tracking system.

To me its beautiful, because it does what an issue tracking system does not do: it explains everything. Who, why, how, what, when. It is beautiful and simple documentation.

Issue tracking typically revolves about the who, what, when - not why it happened, or how it was resolved.

This is why I believe that code can never be fully self-documenting. I can't understand why the code exists from reading it. All the floofy contextual stuff is missing, and commits like this help to explain the floofiness.

Re: My Favourite Git Commit

#83

Post by simonh »

My favourite Github commit was someone removing their password from a test list in a penetration testing tool, because they didn't want anyone to know their password. I just tried, but couldn't track it down. The subsequent comment trail was hilarious.

Re: My Favourite Git Commit

#84

Post by gus_massa »

leejo wrote: post #45
strictfp wrote: post #30

OR you could just write Replace invalid ASCII char. Fixes rake error 'invalid byte sequence in US-ASCII'. I don't want your entire life story in my commit log.

> I don't want your entire life story in my commit log. I[1] want enough debug information in the commit log to be able to reproduce the issue without having to go on web hunts to understand the problem. Especially when the change appears to be trivial on the surface, because these are the ones that can turn out to be rabbit holes. I don't want to have to interrupt you to get this information because you didn't write…

I prefer that the commit includes the addition of a test in the test suit that get fixed (or a few). This is good because:

* It ensure that the bug is real. [1]

* It ensures that the bug is fixed. [1]

* It prevents reversions (assuming the test are run automatically).

* The test may prevent reversions in other related code, or discover other hidden bugs.

* It brings you closer to a 100% test coverage.

* You don't have to guess how to reproduce the bug, reading the comment.

* If the bug depends on subtle configurations, they should be set in the test. [2]

From time to time there are bugs that are obvious in the code, but they are too difficult to find a test for them.

[1] Been there, done that.

[2] Once I found a bug that depended on the local timezone.

Re: My Favourite Git Commit

#85

Post by bloak »

hleszek wrote: post #10

To avoid those kind of issues, non-ascii characters are forbidden in our code base. They are automatically verified in a pre-commit git hook.

If you don't allow non-ascii then you may have to mangle some people's names in the copyright header.

It's good to check files for unexpected characters, though. Here's some Perl to do it:

  perl -e 'binmode(STDIN, ":utf8"); binmode(STDOUT, ":utf8");
  foreach (split(//, join("", ))) { ++$c{$_}; }
  foreach (sort(keys %c)) { printf "%8d %s\n", $c{$_}, $_; }'
I tested that on a text file I was working on ... and I discovered that the file contained a BOM (U+FEFF), not at the start of the file, but at a random point in the middle of the file. I've deleted it. Who knows what problems it might have caused for me later?

You could have a pre-commit git hook that refers to a whitelist of allowed non-ascii characters, or also allow all alphabetic characters, or something like that.

Re: My Favourite Git Commit

#86

Post by MrGilbert »

leejo wrote: post #45
strictfp wrote: post #30

OR you could just write Replace invalid ASCII char. Fixes rake error 'invalid byte sequence in US-ASCII'. I don't want your entire life story in my commit log.

> I don't want your entire life story in my commit log. I[1] want enough debug information in the commit log to be able to reproduce the issue without having to go on web hunts to understand the problem. Especially when the change appears to be trivial on the surface, because these are the ones that can turn out to be rabbit holes. I don't want to have to interrupt you to get this information because you didn't write…

This. It's the same with comments in code:

I don't want to read what the code does (I can read that myself, thanks!), I want to know WHY it does it the way it does it - especially, if there is a more obvious, better way.

Also: People leave companies. Or die. At some point in time, you won't be able to ask the original author.

Re: My Favourite Git Commit

#87

Post by drej »

tigershark wrote: post #63
drej wrote: post #41

I love these commits. Then don't have to be this verbose, but they have to tell a story of why things were done. I can sort of deduce the what from the code itself, but the why is sometimes shrouded in mystery. I started with these explanatory git commits a few months ago and they are super useful, even if you're just reading your own commits from some time ago.

To me they are red flags because it means that very likely who writes this kind of commit messages doesn’t use a proper issue tracking system.

I use these commits, but also use a proper issue tracking system. So I'm not quite sure your comment applies. The reason I'm doing this is:

1. If I'm looking at some code, I want to see its history without having to switch between git(lab|hub) and jira or whatever system I'm using.

2. The issue tracking system doesn't necessarily have some kind of resolve, a summary of what and why happened. It does have a description and a series of comments, but a summary is usually lacking.

3. I believe my commit history will far outlive any issue tracking system I use. So I'm safer putting information into both.

Re: My Favourite Git Commit

#88

Post by 0zymandiass »

leejo wrote: post #45
strictfp wrote: post #30

OR you could just write Replace invalid ASCII char. Fixes rake error 'invalid byte sequence in US-ASCII'. I don't want your entire life story in my commit log.

> I don't want your entire life story in my commit log. I[1] want enough debug information in the commit log to be able to reproduce the issue without having to go on web hunts to understand the problem. Especially when the change appears to be trivial on the surface, because these are the ones that can turn out to be rabbit holes. I don't want to have to interrupt you to get this information because you didn't write…

To me, at least, the issue with that commit message is the signal-to-noise ratio. There is a lot of exposition for each piece of information. I prefer a more declarative commit message. However, from the writing, I suspect this is just due to the committer not being a native English speaker.

e.g. the first paragraph doesn't lose any important information trimming it down to:

"After adding a test matching the contents of router_routes.conf, `bundle exec rake` fails with:

    ArgumentError:
        invalid byte sequence in US-ASCII
"

Realistically, it would have been a better commit message if they'd given the shortlog SHA where the test was added that exposed the bug rather than an explanation of what the test does.

"After adding test (08c3e17), `bundle exec rake` fails with:"

Re: My Favourite Git Commit

#89

Post by DJHenk »

strictfp wrote: post #30

OR you could just write Replace invalid ASCII char. Fixes rake error 'invalid byte sequence in US-ASCII'. I don't want your entire life story in my commit log.

That's why most guidelines for commit messages prescribe a short description and an optional long description. The message in the article does not have a short description, which would have been easy to include. For that reason, it's not "My favorite Git commit message" either.

Re: My Favourite Git Commit

#90

Post by segfaultbuserr »

Also, if you haven't seen it before, read the Linux kernel Changelog. The latest Changelog can be found at [0]. Almost every commit tells a story, unless it's a trivial fix. If there's a bug, it often contains detailed analysis and rationals, and it's a form of important documentation.

Although it's not always practical to follow them in personal/work projects - Linux commits are the results of multiple rounds of reviews, and the commit log is its justification - but in personal/work projects, commits are made in real-time as soon as you debugged/refactored something. But I still use Linux kernel as a guideline for my own commit log, at least for new features or bugfixes.

[0] https://cdn.kernel.org/pub/linux/kernel/v5.x/ChangeLog-5.3.7

Post reply on HN