6 ms·

Yep, no one ever reads them and the cost to benefit ratio is extremely low. Writing good comments is far more important. Don't explain what the code does - tha

• 0xEFF 6y ago
  I look at commit messages quite often working on a 10+ year old code base. Bisect leads to the commit, the commit message explains why the change was made. I take the explanation from 10+ years ago and determine if the reason still applies today. Or, I bisect and there's a low quality commit from 10 years ago from a person long since departed that simply says "fixup" and I'm dead in the water.
  • systemvoltage 6y ago
    Commit messages are supposed to be short. "Fixed stuff" is totally wrong. I usually write "Added ability to do foo with bar when baz is true." Commit messages aren't mutually exclusive to the inline documentation. I am making the case that inline documentation is far more important than commit messages.
    • jkaplowitz 6y ago
      There's no general rule about what length a commit message should be, except that often people try to keep the first paragraph as a single short line for better readability in systems like GitHub and the git CLI. When additional detail is helpful about the motivation for or gotchas related to a commit, starting a new paragraph and elaborating there is totally fine. But I agree with you, they aren't mutually exclusive to inline documentation. The inline documentation is more important when the explanation is relevant to understanding the code which results from the commit, whereas a clear commit message is more important when the information is to illuminate the reasons for (or history surrounding) the change itself.
      • systemvoltage 6y ago
        Agree with all you've said. Commit messages do serve a slightly different purpose of historical context.
      • cwsx 6y ago
        I was taught by a previous manager always to follow: `add/update/remove/fix {feature/bug} by/with {reason} ({further explanation if required})` This also follows what I've seen at a few companies since then. Would you say that's a general rule?
        • MaxBarraclough 6y ago
          There's now an effort to (loosely) standardise this convention: https://www.conventionalcommits.org/en/v1.0.0/#summary https://www.conventionalcommits.org/en/v1.0.0/#summary
          • jkaplowitz 6y ago
            Ah! That seems slightly different from the format the person you're replying to described, but I have seen this version in use at one job. There was only used by one person who gave no context on why he used that format, so it's probably no coincidence in such an example that to me the extra structure seemed to add only opacity and no real value. I can absolutely imagine that being different in a company that used this convention widely and built tooling around it. Doubly so when a lot of the staff is junior enough that it's helpful guidance for structuring thoughts around what the commit does.
        • jkaplowitz 6y ago
          I've never seen that anywhere I've worked, though it's a readable enough format that it wouldn't bother me if someone decided to adopt it for a good reason like compatibility with parsing by dev workflow dashboards or other automatic tooling. Though, I'm not sure it's a helpful restriction for commits that clarify, refactor, or clean up code. One can certainly reference an issue tracker number as the {feature/bug} element that gives the necessary context, but the strictness spreads the information around more widely than is natural. If someone was adopting that convention as a workaround for helping team members learn how to communicate, they're probably going to find that it's a very incomplete workaround.
    • xorcist 6y ago
      "Supposed" to? Git was made with the intention that commit messages look like whole emails, describing not only why the change was made but also the thought process behind it, why this particular solution was chosen instead of some other etc. What you describe is a decent header, although most people would probably prefer "Add ability to" (not "Added ability to", this is a custom that goes way back before git). Inline documentation, or comments, is something else entirely. That is a moving target that can describe intended usage, remote APIs, and hard to understand passages. Commit messages describes a particular changeset, at a fixed point in time. Google it, and see that most people's ideas of a good commit message goes way beyond the header ("shortlog") line.
      • alexchantavy 6y ago
        GitHub discourages making commit messages longer than a certain number of characters; it’s interesting for me to see other opinions on that.
        • andersonvom 6y ago
          You're talking about the title of the commit message. Yes, the title is supposed to be under 50 characters. Now add a blank line after it and you can write all you want (line wrap at 72 chars). If you use markdown Syntax in your commit body, github and other tools will gladly render it correctly. You can grep for commit messages with "git log --grep <pattern>" and it will also search commit bodies as well. It's beautiful!
        • smichel17 6y ago
          Adding on to sibling comment, on GitHub the additional lines they're talking about go in the box before the commit title.
      • alphachloride 6y ago
        Could be selection bias. People who comment about good commit messages are most likely people who pay attention to commit messages. That does not necessarily mean they make up a majority of committers.
    • mcv 6y ago
      Commit messages and code comments serve different purposes. The commit message should be about what you changed, so they help you find the interesting commit from a list of commit messages, whereas comments in the code should be about why the code is the way it is. Is it because of a bug in a library? Is it arbitrary? A quick fix that still has some debt? Did someone waste a lot of time on this? Commit messages are about the change, whereas comments are about the state of the code.
  • oefrha 6y ago
    Yup, way too many times I’ve encountered code that’s written in a peculiar way obviously for a reason, most likely to work around some issue, but it’s not clear what the issue is and whether it is otherwise resolved so that the peculiar code can be dropped. I bisect and it says “fix bug”, now I need to carry that crap forever or worry about reintroducing some unknown bug. (In case someone points out a comment is more suitable: comments are less likely to survive refactors, and people who write terrible commit messages also tend to be terrible commenters. Informative commit messages supplement good comments.)
    • heavenlyblue 6y ago
      Why is the reason to peculiarity not in the comments in the code itself?
      • usrusr 6y ago
        Comments aren't supposed to state the obvious and at the time it probably was. Commit messages are redundant by definition (everything said in the message is supposedly already there in the diff) so they are like magnets for stating the seemingly obvious.
        • u801e 6y ago
          > Commit messages are redundant by definition (everything said in the message is supposedly already there in the diff) Commit messages are far easier to parse and understand compared to a moderate size diff. By reading the commit message before looking at the diff, it really helps in terms of understanding the context of the diff and what to expect.
          • greyhair 6y ago
            More than that, I find moderately sized 'git' diffs in PRs to be unreadable. I frequently have to open the full code block to see what is taking place in the source. Commit messages provide intent, and should reference the original change request to see what the commit was intended to fix. I would say the whole chain matters. A good change request (Jira) with defects, and how to recreate the fault, as well as relevant requirements (by reference, not cut-n-paste) Good comments on the code that is changing. Don't document the defect and how you fixed it, just comment what the code does (if the code is not perfectly clear), Then a good commit message, referencing the change request, to tie it all together. It all matters.
• Mekantis 6y ago
  I think the issue with comments is that they lose all temporal and situational context. You see a comment next to code, but you have no idea if it's actually still addressing that piece of code. How out of date is that comment? Nobody knows. But if you look at the git history of all the edits specific to a part of code, you'll have a full view of what happened and why and by whom. I think the big issue is that programmers aren't taught to properly leverage git in this holistic way - instead it's just "version control", when it can be such a crucial and informative and helpful part of a process around documentation, writing good code and working in a team.
  • RealityVoid 6y ago
    > I think the big issue is that programmers aren't taught to properly leverage git in this holistic way You are assuming here that all programmers even "do" git. A lot don't. A whole lot of them. I worked with some companies that would have some horrible horrible version control system only and I just wanted to cry. Sure, git has its warts, but once you get it, it's a tool that can be _incredibly_ productive. I hate having good tools taken away.
• Matthias247 6y ago
  > Yep, no one ever reads them and the cost to benefit ratio is extremely low I read them quite often. Whenever there is a regression detected between 2 software revisions that have been deployed somewhere the first thing I do is checking commits and commit messages for packages that have changed to find the more obvious reasons for a behavior difference. Checking code diffs at that point would be a lot more effort.
• u801e 6y ago
  > No one ever looks at the commit messages when trying to figure out what the code does as it convolves history and state of the repository Do people not use git blame to check what they're changing before they change it? That's how I check the history around the changes I'm going to make before I make them.
• phkahler 6y ago
  >> Yep, no one ever reads them and the cost to benefit ratio is extremely low. You're projecting. YOU never read them. I used to be the same way. Now I see other people reading them and I've started to myself. I follow other people's efforts and progress. It's a great way to learn not only the code but to communicate better.
  • alphachloride 6y ago
    Yes, in the literal sense "no one ever reads them" is obviously false as you provided contradictory evidence. But I find it equally obvious that the commenter meant it figuratively. Is there any data to support your or his claim as more representative of the population?
• phlakaton 6y ago
  I can't say I _read_ commit messages all the time. I do however, _look_ for commit messages all the time. And am frequently disappointed. The comments I put in commits are things you'd be horrified to see littering your codebase (though I did it that way once upon a time too). They are the "whys" behind a change. Sometimes they have relatively little to do with individual lines of code, and don't need to be maintained like comment blocks should be. The temporal binding is absolutely intended.
• greyhair 6y ago
  Comments and commit messages both matter. For different reasons. Comments are often wrong, for the wrong reasons. People that change code, but don't update the comments to reflect the code change are evil. But then, people that put to much superfluous information in comments, that should just be left to the code, are also evil. BTW, if your code implements a part of a standard, try putting the standard, and what subsection, your code addresses. Just the reference not the text. 802.11G for example (old fart) or TA55, or RFC 1536. Whatever. Then list the date of the document and the subsection. I had to modify a Reed-Solomon implementation many years ago, and the original two authors based the implementation on a particular textbook. They gave the ISBN and then chapter/paragraph/table/illustration references on each block. I went and got a copy of the book, and it all lined up perfectly. Best documented code I have ever encountered.