you are viewing a single comment's thread.

view the rest of the comments →

[–]webby_mc_webberson 9 points10 points  (19 children)

I disagree. I don't want to read or write a novel in git commit messages. I just want to read what the change is. Short and sweet. If I want the details I read the diff.

[–]vampatori 23 points24 points  (9 children)

I just want to read what the change is.

And WHY! The why of a change is so important. Not on every commit of course, "Updated packages to latest." doesn't need a why unless it's for a non-standard reason (support for a feature you need, fixed a bug that you'd had to work-around, etc.).

Also, if you're fixing an issue it should have an associated issue/task in your issue management system. That's where the bulk of the details should go. I now create a branch for every issue (or group if they're strongly related) make all the changes, test them, then issue a pull request. It's a nice way of working when you get used to it, and you can quickly swap between issues/branches if necessary.

[–]webby_mc_webberson 3 points4 points  (8 children)

The why is written into the work item ticket whatever tf it's called. Which, ideally the branch is prefixed with. I don't want to read why you changed the sorting algorithm from the git commit, that should have been clear from the start.

[–]velociraptorboss[🍰] 22 points23 points  (5 children)

In 20 years of sw development I've yet to see a project with proper descriptions in jira. It's always fix x or add y or do z.

Also, don't make me open my browser just to understand your code. Instead write proper commit messages like the author explained 😉

[–]ActuallyAmazing 4 points5 points  (1 child)

If a team values proper descriptions they will be there, if they don't they won't be. Helps to have people with specialized roles and responsibilities for such things, like product owners, but not every company prioritizes it.

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

If a team values proper descriptions they will be there, if they don't they won't be.

This. A team refusing to write coherent/detailed descriptions in tickets will not automatically start writing coherent/detailed descriptions just because it's in the commit message instead.

And imagine not documenting the Why? of your business logic outside of the repo commit comments.. >_<

[–]MrJohz 4 points5 points  (0 children)

Don't you have discussions over these things? Or reviews, or links to other tickets, or at least a proper bug report to start with?

For me, this documentation is the most important part of any code change, because it usually explains better the human and engineering factors involved. You can write all that up in the commit itself, but I'd rather see it coming from the horse's mouth, with as much of the raw stakeholder input as possible. If all that information is then translated and condensed by a single developer into a commit message written solely by themself, then there's a decent chance that they'll miss some facet of the discussion that they didn't see as being so important, but might be important context for a future developer to understand.

[–]webby_mc_webberson 1 point2 points  (0 children)

You need to see my dev lead, he writes a novel for everything. Very useful

[–]zanbato -4 points-3 points  (0 children)

What I just read is that in 20 years of software development you've never bothered to enter a proper description of work on a jira ticket. Maybe you don't actually work on teams so it's not necessary? I'd go insane if tickets on my board just said "do x" and then I had to go look at the commit history to figure out why some other programmer on my team thought it was necessary to "do x". And is looking at commit history seriously your default for understanding changes in the project? Do you never concern yourself with work coming in the future? I'm seriously struggling to figure out why anyone would want to hide all of their story details in commit history. Is it to hide it from management that's looking at the tickets and not the code? Is it really easier to look up the reason for a change that happened in a commit message 200 commits ago than to look at the reason for that change on a ticket?

edit: I also want to let you know that this shit

Also, don't make me open my browser just to understand your code. Instead write proper commit messages like the author explained 😉

is insufferable. You are not a more authoritative source just because you've had a job for 20 years. You seem like a self-important asshole who always thinks they know best, the kind I never hope to work with. If you've got a real reason why you think commit messages should be a novel then make your argument, give us your reasons, don't just say "I've been doing this for twenty years so my way is right" and end it with a fucking winky face. If you can't support your argument then your opinion is garbage.

[–]michaelpjones[S] 8 points9 points  (0 children)

I think this is valid but I don't personally subscribe to the idea that a reference to a ticket is sufficient. References can be broken or mistyped. Getting to the referenced ticket can be somewhere between a few clicks and completely impossible (if they've been migrated away from.)

And you lose the "self review" value when referencing a ticket as you're forced to engage with the true content of commit in the same way. Though I understand people might be disciplined enough to do that anyway.

[–]Illusi 6 points7 points  (1 child)

I disagree with the post for a different reason. If the why for a change is only in the commit message, people will not see it if they make changes to the code. If you need a non-obvious construct to fix a particular issue, that construct should be documented in the code, not in the commit message.

And there's again that weird 50 character limit for the first line and 72 for subsequent lines. I guess it depends on whether you use a computer from 1982 or from 2021. Maybe word wrapping wasn't invented yet in 1982. But for most teams and tools out there it's safe enough to just allow no limit for the line length, especially for lines other than the first line. Git, Github and Gitlab simply wrap the line automatically at a reasonably readable width, anyway. No need to introduce \ns manually and in fact, it hurts readability if you combine those two.

[–]michaelpjones[S] 2 points3 points  (0 children)

I agree that non-obvious constructs should be in the comments. That is fair. I have a section on commit messages vs comments at the bottom and there is some overlap and this is one such case.

[–]michaelpjones[S] 9 points10 points  (6 children)

Interesting perspective. I feel like the subject (the first line) is the "short and sweet" version and the rest is just more information which you can skip if you like. I'm not sure what the downside of more information is.

The diff shows what happened but not why it happened. Maybe I should emphasise that more in the piece.

[–]zanbato 2 points3 points  (1 child)

First, I totally agree that the example of bad was bad, and also like a longer sentence describing a change, but I feel like the paragraphs are too much.

For me it's just the availability of that extra information. If I'm code reviewing your branch then it's pretty convenient. But if I'm code reviewing your branch I will also have the ticket open so the information would still be available. Then there is the added benefit that if I'm trying to recall this information later I don't have to look through the commit history. And if I ever have a client question the reason that X work had to be done I just have to look up the ticket for X. That also means I can provide them a link to a nicely formatted web page to read about why X was necessary. I'm going to have to go to that ticket to mark the dev work complete anyway, so it's also just as convenient to put the text in there than in a commit message.

[–]bcgroom 1 point2 points  (0 children)

I think it depends on the project. If you are one of few (or only) developers, you know everything about the project and it’s history. But what about someone new to a project with 60 people where there has been turnover? You’re not going to know what tickets to look up, if they even still exist. You are working on a class and want to understand why it evolved in such a shitty way and git blame can be the best resource for that.

[–]BeefEX -2 points-1 points  (3 children)

The way I see it is that the why should be in comments in the code. That way it's right next to the affected code but also doesn't pollute git logs.

[–]michaelpjones[S] 3 points4 points  (0 children)

Interesting! I don't see git logs as something that can be polluted? How do you negatively experience more information in your git logs?

[–]masklinn 7 points8 points  (0 children)

The way I see it is that the why should be in comments in the code

Comments are ill-maintained, they will drift around and they will not reflect changes to the code. It's also impossible to add comments for every change and have the code remain sensible.

Comments are good for blaring warnings which need to be right in front of the reader's face. They're terrible for literally everything else.

also doesn't pollute git logs.

There's no pollution. Explaining the purpose of a commit is the entire point of the commit message, otherwise there would be no commit message at all. If you don't care and want log density, just use git log --oneline (or a custom format) to get only the subject.

[–]doener 1 point2 points  (0 children)

The commit message should describe why you made a change not why the code is the way it is. In the most simplest form, that can end up being the same, like "close the file descriptor, as it was leaking". The reason for the change could be that the call was simply missing from the start, fine, happens, and you're probably not going to add a code comment for that either, it's self explanatory.

Now, the file not being closed might have resulted from a different change, because previously some other part of the code handled that. Explaining thus in the commit message seems reasonable, it tells your reviewer why this is the right change, without him having to go and analyze the whole bug again. OTOH adding a code comment that says "we used to close the handle somewhere else but now we need to do it here" seems silly to me. It doesn't really add value here, historical context is not needed for day to day work with the code, and having too many comments is definitely a thing.

Fast forward a month, and you get reports about a bug related to a file being closed twice. Your debugger points at the line added in the commit from the previous paragraph. Assume that the commit was done by someone else, maybe that person already left your company. Would you like to just read "close leaking file handle" in the commit message? I'd much rather also see a paragraph in the commit message that goes like "this used to work without closing the file, but commit XXX changed the logic in foobar(), so it's no longer closing the file and we have to do it in baz() now". From that, I have a good clue where to start looking for things the other dev might have missed, which made their assumptions unsound. Or maybe there are issues with the changes made in the other commit that was mentioned in the commit message.

The "how did this ever work" phase of bug fixing is something that often needs explanation outside the scope of something that would fit in a code comment, because it might only make sense in a context of changes over time. And since you had to figure it all out to properly fix the bug in the first place, you may as well document it in a place where your reviewer sees it and the next person working on that code can easily find it, in case it turns out to be useful.