• apigban@lemmy.dbzer0.com
    link
    fedilink
    arrow-up
    9
    ·
    1 year ago

    Hello! I’m a hobbyist in this space (scripting/coding), does anyone here have a:

    • gold standard of what commit messages should look like?
    • common practice/etiquette for commit message?

    I never had a team or guide or mentor and when I saw this i felt that my commits are like smoke signals describing that there’s a fire. which isnt really helpful.

    I tried to contribute to a python module that I use daily, my PR was so over engineered (iirc i added just 3 lines, but with tests, screenshots, CI/CD) i think to compensate for my lack of experience that I got called out (“wow this is pretty extreme just for that feature”).

    • barsoap@lemm.ee
      link
      fedilink
      arrow-up
      3
      ·
      edit-2
      1 year ago

      The gold standard, as so often, is to consult an oracle able to tell you what questions you will be asking when looking at the comment or commit in the future. Then answer those questions and write them down.

      In lieu of an oracle, use your experience and best judgement.

      Oh and never write whole papers to explain what you’re doing, unless you’re actually writing a whole paper. Instead, drop a cheeky doi;// URI as the only comment of the whole file to document that you’re an experienced enough programmer to copy from papers instead of stack overflow.

    • bleistift2@feddit.de
      link
      fedilink
      arrow-up
      1
      arrow-down
      1
      ·
      1 year ago

      My personal cent: Some tools strongly suggest that your commit messages should not exceed 50 characters in the first line, and 80 characters on every other line. While the 80-character rule makes sense if you’re using a terminal (and someone on your team will even if you don’t), I strongly disagree with the 50-character rule. If you want to be in any way clear what you did, 50 characters is simply not enough even for the subject line.

      • CoderKat@lemm.ee
        link
        fedilink
        arrow-up
        3
        ·
        1 year ago

        The reason for the approximately 50 character limit is because there’s many tools that display a single line and will truncate it if it’s more than about that length (though really the point of truncation can vary wildly – plenty of tools will let you go twice that before they cut you off). So if your one line summary is too long, it’ll be cut off and harder to understand your commit at a glance.

        You always can elaborate in a second paragraph, at any rate.

        • bleistift2@feddit.de
          link
          fedilink
          arrow-up
          3
          arrow-down
          2
          ·
          1 year ago

          How much can you really put in 50 characters?

          Fix: NPE in customer download component when users – That’s 50 characters. Should I not mention where I fixed the bug?

          Fix: NPE when users downloaded customers without s – I think I can get rid of the actor in some cases.

          ̀ Fix: NPE when downloading customers without select`. The summary I want to give cannot be truncated any further.

          Fix: NPE when downloading customers. This fits, but is so vague as to be pointless as a summary, in my view.

          • brettvitaz
            link
            fedilink
            arrow-up
            4
            ·
            1 year ago

            I don’t follow the 50 character rule, but to answer your first question: imo, no, don’t mention where you fixed the bug. This is a commit message that is explicitly tied to the place where you fixed the bug. You can go into more detail in the PR if you need to.

            • bleistift2@feddit.de
              link
              fedilink
              arrow-up
              2
              ·
              edit-2
              1 year ago

              “PR” lol. I’m the sole developer.

              Jokes aside. I find it easier to have all the information in a commit message, so I can browse it in the git log without having to find the relevant PR(s) that finally merged it.

          • barsoap@lemm.ee
            link
            fedilink
            arrow-up
            3
            ·
            edit-2
            1 year ago

            Fixes #23132: NPE when downloading customers

            Reading that you see:

            • It’s a bugfix, not feature addition or such
            • The actual ticket number in case you need to dig deeper.
            • A quick blurb about what and where. The rest is in the code. If the issue title is good you often can simply re-use it (but not always).

            It’s plenty to orient yourself in case you need to dig in, plenty so that you can skip over the commit if you’re looking for something else, and short enough to ignore. If you absolutely need to document more, do it under the fold but that headline there is what should show up in a list of commits.