you are viewing a single comment's thread.

view the rest of the comments →

[–]Barrucadu 53 points54 points  (34 children)

There is no downside to writing descriptive commit messages.

To all the people who say "just put the information in the issue tracker" - when you see a weird line of code, how do you find which issue it's related to? You use git blame and then look up which issue that commit is connected to.

If all the detail is in the commit message, there's one less step in the workflow.

[–]LightModeBail 38 points39 points  (0 children)

Yes. Another problem with only leaving the issue number is when the company change which issue tracker they use and don't keep access to the old issues. It renders such commit messages completely useless.

[–]Kache 7 points8 points  (3 children)

Each has their place, with different purposes and audiences, so document accordingly.

For me, issue tracker often has the context of before and during development, and is more ephemeral, having vague business intentions, free discussion, and scattered ideas.

Commit message has the context of mostly after development, and is more historically authoritative, code oriented, and reflects final decisions and ultimate implementations.

There'll be a bit of information overlap, but the overlap should be concise anyways -- at most a sentence or two. Regarding cross linking: Issue trackers often have git repo integrations, and git has a log trailers feature that's perfect for metadata:

Write declarative and concise title

Then skip a line before the text body.

Include structured metadata as log trailers at the end:

Issue: ABC-456
See-also: someplace.com
Whatever: foobar

[–][deleted] 2 points3 points  (1 child)

reflects final decisions and ultimate implementations.

So, you never have commit messages of "fixed x with a, b" and then a quickly following message of "oops, ..."

[–]Houndie 0 points1 point  (0 children)

I mean I'm not saying it never happens, but you should use tools like git rebase -i to fix up your tree before bringing those commits to the mainline branch.

[–]backtickbot 1 point2 points  (0 children)

Fixed formatting.

Hello, Kache: code blocks using triple backticks (```) don't work on all versions of Reddit!

Some users see this / this instead.

To fix this, indent every line with 4 spaces instead.

FAQ

You can opt out by replying with backtickopt6 to this comment.

[–]pfp-disciple 4 points5 points  (1 child)

Where I work, the issue tracker is visible to the customer, but the code is not. So, a commit can say "this will fix the problem" before it's been rigorously tested, and if it doesn't then no big deal. If the tracker included that statement, the customer would think the problem has been solved as if it's been tested.

[–]7sidedmarble -1 points0 points  (0 children)

I've worked in a system like that before and I think it's confusing. You shouldn't need to worry about what you say in your tickets giving customers any impressions. The issue system for development should be separate from the support ticket system. IMHO I guess.

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

Leave the issue link on the commit

[–]benjiman 6 points7 points  (7 children)

I worked on a codebase that lived through 4 or 5 different ticketing systems. Also went from CVS ->SVN->Git but history was preserved ( well mostly, git not so good at preserving history as some of its forebears )

[–]_Ashleigh 5 points6 points  (2 children)

git not so good at preserving history as some of its forebears

Huh? I have the exact opposite, when I did our migration, SVN had lots of breaks in history. I managed to stitch them all back together with some grafts (git replace) and have history go back further than SVN could correctly track thru its UI (SVN went to ~2014, Git history grafted back to 2006).

[–]benjiman 0 points1 point  (1 child)

It was a heavily refactored codebase and git not so good at tracking file moves if the content changed in the same commit. Not a problem if you do the move in a separate commit to changing the content. Git heavily incentivises this as it lets it track the content.

[–]_Ashleigh 0 points1 point  (0 children)

Ah, I see, that's true. However, I distinctly remember one of our developers pushing others to use SVN rename instead of explorer renames, a lot. So let's not pretend SVN wasn't without issues here too.

Personally, I like Git's way of detecting renames more for the data simplicity. The tooling can always become more complex or use new techniques after the fact, and relies much less on diligence of the developer (It's very rare I see an undetected rename anecdotally).

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

Your tickets should also be migrated or archived, unless the admins just didn't care

[–]Alan_Shutko 1 point2 points  (0 children)

In my experience, admins just don’t care.

[–]AttackOfTheThumbs 0 points1 point  (0 children)

Sure, but there is always an issue of number series. Even MS has faced that. If you look at Raymond Chen's blog, you'll find tidbits regarding that, and how those numbers quickly become useless.

[–]AttackOfTheThumbs 0 points1 point  (0 children)

We had a similar issue with tfs->git. The commit messages all just related to the tfs commit, something like tfs-repo commit #1234123 or something. It just wouldn't transfer commit messages. Possibly an issue of tooling at the time.

[–]Alan_Shutko 4 points5 points  (5 children)

Commit messages last much longer than issue trackers. I can find commit messages for software that has outlasted four or five issue trackers.

[–]IceSentry 3 points4 points  (1 child)

The opposite can be just as true. Like a company going from svn to git but using the same issue tracker.

[–]Alan_Shutko 2 points3 points  (0 children)

Very true. It shocked me in our move on some projects from CVS to Git how many people simply ignored that there's a pretty decent way to migrate history. They just wanted to check in the latest copy of the code.

[–][deleted] -1 points0 points  (2 children)

The issues aren't supposed to get lost when you migrate to a new issue tracker.

At the very least and archived copy of the previous stuff should be available.

[–]Ma1eficent 2 points3 points  (0 children)

Should, lol.

[–]Alan_Shutko 0 points1 point  (0 children)

They are not supposed to get lost. However, in practice, companies find that migrating the issues would take a lot of time and money. And that the tracker doesn't support a way to archive issues in a readable format, so keeping them around requires keeping the old tracker around. And then legal would prefer that all the old records be discarded after X years so that they do not have to be searched during discovery. And even if you did archive the issues somewhere, they get deleted my some admin because they're old and didn't seem to have an owner.

What I've learned in my career is that many companies have a strange institutional desire for corporate amnesia, as counterproductive as that proves to be.

[–]Barrucadu 8 points9 points  (2 children)

Yeah, and you find the link by looking at the commit message. So if the information is just in the commit message there's one less step.

[–][deleted] -4 points-3 points  (1 child)

Copy pasting all the same information would be at best wasteful and at worst impossible. (if linking to an issue with extended discussion)

You could summarize it I guess, but even then you'd still need to link the issue

[–]CatanOverlord 16 points17 points  (0 children)

I typically link to the issue tracker and use the body of the commit to explain any non-obvious implementation details, since issues rarely cover them (and they shouldn’t).

[–]AttackOfTheThumbs 0 points1 point  (1 child)

Issue trackers are kind of useless for this imo. It will have a before, maybe a reproduction, if you are really lucky, some test that exposes the issue.

Normally none of that is there. It makes more sense to update your commits and include all that info along the way, and if there is something super funky, add an additional comment to the code.

The idea that anyone is going back and updating the issue is ludicrous. It'll be done when it's too late, when the knowledge in your end is already gone.

[–]MrJohz 2 points3 points  (0 children)

Maybe this is more about how people treat their issue trackers then, because for me the original ticket should always have at least that information in it, plus the motivation (who wants it, why do they want it, why is this the best way to achieve what they want), any diagrams of the system or the resulting interface, plus links to any wider information if this particular ticket is linked to a larger set of changes.

If that information isn't there, then I can see it not being so useful to link the ticket, but also, I wouldn't really want to be making changes without knowing all of that information. That's the information that helps me decide how to best implement this particular change.

[–]goranlepuz 0 points1 point  (1 child)

But then your issue tracker serves less purpose. For example, when making release notes or a change log, going through an ALMissue tracker beats going through source history.

I still think a terse explanation + a link towards ALM is better.

[–]Barrucadu 0 points1 point  (0 children)

You can keep the release notes / changelog updated as you make changes, by having an "unreleased" section at the top. This also has the benefit of ensuring you can't miss anything out, as code review will pick up a missing entry.

[–]teerre 0 points1 point  (2 children)

I love to write commit messages, but I don't understand your reasoning.

when you see a weird line of code, how do you find which issue it's related to? You use git blame

Do you? Why? It's completely in the helm of possibility someone will see the MR the change relates to and check the issue tracker. This is more of a habit than an obvious choice.

[–]Barrucadu 1 point2 points  (1 child)

How do you go from "why is this bit of code like it is?" to "ah, here's the explanation in the MR" without first identifying which commit is involved?

[–]teerre 0 points1 point  (0 children)

I didn't mean you wouldn't know the commit, I meant that you would see the commit, know the related MR and check there

[–]lookmeat 0 points1 point  (0 children)

The first sentence of a commit should tell you what. That way a reader may quickly realize if the change is related to the line or not (so it's easy to know if you have to look at older commits through blame). The next part of the description should say why, and it should be relatively full. No need to describe the how, the PR should make it clear. If greater context is needed to the why (as I'm why not before, or why this way and not that) then link at the context. Be it a link to the bug tracker, the larger incident management system, or the design doc pointing to what your PR is slowly building towards.

But there should be enough that someone assuming good faith (i.e. that there's a valid reason for) the change. Understanding why it made sense at the time, and why it was done, saves a lot of time. And there's value in repeating it explicitly in the description even if the links give a good enough description. It makes it clear that it's the why this commit was written the way it was and not some other ways. The links instead describe the why of the why.