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

Rob Pike on good commit messages (2014)

groups.google.com

Post reply on HN
Print view View original

71–80 of 98 posts

Re: Rob Pike on good commit messages (2014)

#71

Post by Vysero »

It seems like what was said by Austin Clements was more of a bug report than it was a description of a solution for a bug. Which is the job of reporter and not the person fixing the problem.

In my mind, a commit message should not be the be all end all for what can be said about a particular bug fix. If someone needs to know the details then they can look at the diff and if the person who made the change feels it was complex enough then they should add a comment on the bug report about what was changed and how it was changed prior to marking the bug as *fixed.

Re: Rob Pike on good commit messages (2014)

#72

Post by yodsanklai »

stefan_ wrote: post #60

Then single best commit messages you are ever going to find in any given project are in the Linux kernel: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/lin...

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/lin...

Wow, that's a great commit message for one character change!

Re: Rob Pike on good commit messages (2014)

#73

Post by fdr »

kstenerud wrote: post #62

That's a lovely description of why the fix was needed, but I just took a look at the go source, and NOWHERE IN THE SOURCE CODE IS THIS FACT DOCUMENTED. Signals are tricky, temperamental beasts; exactly the sort of thing you'd want to have comments about in your code. The way things stand, nobody would ever know why the signals are set up the way they are unless they trawled through 40,000 commits. Yes, 40,000: git lo…

Do not agree. Commit messages can be more expansive than comments, because comments bitrot more readily: they are not rooted in a time and change to the code.

I use commits for the largest bulk of explanation, particularly "why," which can be rather detailed and not of much interest to someone skimming the code rapidly.

Comments I use for pointers that are unlikely to bitrot and/or important to prevent error. Except for the most timeless design elements in a project, I do not commit long documentation.

Re: Rob Pike on good commit messages (2014)

#74

Post by bendavis381 »

Pxtl wrote: post #46
SkyPuncher wrote: post #41

Earlier quoted context omitted.

Isn't that what rebasing is for? We rebase all of our feature branches for PR's.

Yeah, I just mean the PR process auto-squashes/rebases instead of merging, so that junior developers don't have to think about it. The problem is that if somebody leaves the branch up (delete-on-merge is obviously preferred), or derivative experimental branches, you can't tell if it's been merged or not because the commit hashes are different after the rebase. The branch list has become a scary basement.

Check out how phabricator handles this. It's essentially what you're asking for: https://secure.phabricator.com/phame/post/view/766/write_rev...

TL;DR: PRs are not based off a diff between master and a remote branch. Instead, phabricator sends up patches to code review and when approved, the Phabricator tooling handles the rebase locally. No remote branches and clean history.

Re: Rob Pike on good commit messages (2014)

#75

Post by RussianCow »

saagarjha wrote: post #59
RussianCow wrote: post #50

Earlier quoted context omitted.

> Limit the subject line to 50 characters It's nearly impossible to get certain points across in 50 characters. I keep mine under 72, and even then, I sometimes struggle to adequately describe a change in that little space, even at a high level, to the point that it would be useful for someone searching for something. Honestly, all of this stuff matters way less than the actual content of the commit message anyway. S…

> It's nearly impossible to get certain points across in 50 characters. Usually that kind of stuff goes in the body of the commit message.

But most Git UIs only show the first line of the commit message when displaying the log, so if you're searching for something specific, you're only going to see that without clicking into a commit. I want that first line to tell me whether this commit is relevant to whatever I'm looking for, and it's extremely difficult to do that in 50 characters.

Re: Rob Pike on good commit messages (2014)

#76

Post by xorcist »

kstenerud wrote: post #62

That's a lovely description of why the fix was needed, but I just took a look at the go source, and NOWHERE IN THE SOURCE CODE IS THIS FACT DOCUMENTED. Signals are tricky, temperamental beasts; exactly the sort of thing you'd want to have comments about in your code. The way things stand, nobody would ever know why the signals are set up the way they are unless they trawled through 40,000 commits. Yes, 40,000: git lo…

> "Moved A to B" is not necessarily bad

There is always a reason why things are 1) done and 2) done in this specific way and not another. It is also relevant if this is part of a larger change and it there are external dependencies on this commit. These things must be clear from the commit message.

> you're just doing janitor work

That makes it sound like janitorial work does not require complete commit messages which is why we sometimes see useless messages. There is always a reason why something was done and this can be documented.

A bug can be documented why it happened, why it was fixed in this specific way and not some other more obvious way, and what steps has been taken not avoid it from resurfacing. The same way a clean up can describe why others paths were not taken, why the new way is considered more clean (fewer lines, lesser complexity?) and how we know it does not entail functional changes.

The git tool is rooted in email, and commit messages look a lot like it. Pretend you are describing your patch over email to someone and it will be almost trivial to write.

Re: Rob Pike on good commit messages (2014)

#77

Post by stinos »

fdr wrote: post #73
kstenerud wrote: post #62

That's a lovely description of why the fix was needed, but I just took a look at the go source, and NOWHERE IN THE SOURCE CODE IS THIS FACT DOCUMENTED. Signals are tricky, temperamental beasts; exactly the sort of thing you'd want to have comments about in your code. The way things stand, nobody would ever know why the signals are set up the way they are unless they trawled through 40,000 commits. Yes, 40,000: git lo…

Do not agree. Commit messages can be more expansive than comments, because comments bitrot more readily: they are not rooted in a time and change to the code. I use commits for the largest bulk of explanation, particularly "why," which can be rather detailed and not of much interest to someone skimming the code rapidly. Comments I use for pointers that are unlikely to bitrot and/or important to prevent error. Except…

I do follow your reasoning, bad comments are worse then no comments, but in this particular case it really feels to me like the meat of the commit body should also be a comment. Just so that when someone looks at the code and wonders, hmm, why is this flag here, he/she can figure it out without having to do a blame/log on the file. (Don't get me wrong, great tool, but has it's own problems).

Judging whether or not comments are permitted on the bitrot-ability of a piece of code seems hard to begin with, and even then a bit arbitrary. In this case, the whole comment would be just for that flag. So if someone really just gets rid of the flag but not the comment, especially on code like this, and that also passes code review then I think there are more pressing problems then bitrot.

Re: Rob Pike on good commit messages (2014)

#78

Post by mjw1007 »

kstenerud wrote: post #62

That's a lovely description of why the fix was needed, but I just took a look at the go source, and NOWHERE IN THE SOURCE CODE IS THIS FACT DOCUMENTED. Signals are tricky, temperamental beasts; exactly the sort of thing you'd want to have comments about in your code. The way things stand, nobody would ever know why the signals are set up the way they are unless they trawled through 40,000 commits. Yes, 40,000: git lo…

I agree that commit messages aren't a good place for detailed documentation.

Another reason is that if you had a mistaken understanding when you made the commit and wrote up that understanding in the commit message, you're creating a greater risk of misleading future readers than if you'd put the writeup in a file which can itself be version-controlled.

I think the right place isn't necessarily code comments; separate internals documentation (in the same repo as the code) can be good too. Then the comment near the code can just be a cross-reference.

Re: Rob Pike on good commit messages (2014)

#79

Post by stinos »

deanmoriarty wrote: post #4

After being in the field for almost a decade now and guilty of so many poor commit messages, I came to the firm realization that a very important trait of a good software engineer is the ability and diligence to write detailed commit messages, and I would have never remotely imagined this during the first few years of my career (I was "raised" in companies who didn't care at all about this and all commits were always…

every single commit I push has an entire paragraph or two clearly describing the changes, and the motivation behind them

Similar here, but: looking back at a couple of years ago I obviously went through a phase where I was just writing complete paragraphs for the sake of making sure the commit looks meaty. So now I treat messages the same as code: code not written is good. If a one-iner is enough to explain why something was done, and the changes in the code speak for themselves, than that's it.

even if it's a one-line diff.

By now you probably figured the size of the diff doesn't seem to have much of a relationship with the amount of commit message needed :)

Re: Rob Pike on good commit messages (2014)

#80

Post by ilikepi »

MayeulC wrote: post #43

I also encourage everybody I know to write descriptive merge commits: "Merge feature X" instead of "Merge someone/master into master". It really helps when using git log --first-parent to avoid displaying intermediate messages. This really helps with the so-called "Aligator" [1] workflow: create a merge commit for every bugfix or feature, instead of just rebasing (you can still rebase if you want). That makes the his…

> I also encourage everybody I know to write descriptive merge commits...

Or alternatively, use a branch naming scheme that includes a little ticket metadata: "Merge branch 'feat_4714_default_tag_filter'"

Post reply on HN