conventions: write good commit messages

[smola]

Add section on writing good commit and pull request messages.
We should be mindful with other developers and users and document changes properly: what and why something changed.

[smola]

Fun fact: I initially opened this PR with an empty description ;-)

[dpordomingo]

lgtm but I'd like to know if we have any preference about PR desc or commit message

[dpordomingo]

Do we have any preference about both (PR desc vs commit message)?

I use to put most of the effort on explaining the change in the PR description because it is easy to link other issues, commits... create examples, headings...
...and less in commit messages (in my case, they're almost a phrase using imperative tense to describe the change to apply)

I know that PR descriptions can be lost if the repository is migrated, or forked, or moved to bitbucket... and commit messages are forever... so, should we focus much more on commit message? or can we rely on GitHub PRs?

[bzz]

(eventually) a lot of people:

and processed by 🤖

[creachadair]

In my view, both are very important. Each commit in the PR should document the change made by that particular commit, and the PR overall should document why these commits are together.

Because the scope of individual commits and PRs is often quite different, the level of detail might vary. However, I would generally argue that they should have a similar structure:

1. A one line synopsis of what the commit/PR accomplishes

2. A more detailed explanation of why and what is being changed.

For tiny commits it may be OK to omit (2), particularly if it's updates from a code review. But even then there can be value—for example:

Updates from code review.

Fix spelling and punctuation errors, and reformat C files with clang-format.

I find the hardest thing about writing good, clear commit and PR descriptions is getting out of the present moment, where I know as much as I will ever know about this particular issue—and putting myself in the future, when someone else will have to reconstruct it. There is no simple formula for that, obviously—what I try to do is anticipate the questions someone is likely to ask me and answer them up front. YMMV.

Most (all?) of our projects seem to prefer not squashing PRs when they are merged to master. I'd argue that it's therefore important that even our little code-review commits speak for themselves. In a repo where commits are expected to be squashed (even if not ahead-of-time), the PR description and the merge commit can basically be the same description.

Either way, though, the general advice proposed in this PR seems good.

[kuba--]

If it's independent PR then good description is required. Generally it's good to have informative description, but I really miss a good description in issues. If I have a good description in issue then it's easier to start and I can add it to PR as a reference (instead of copy paste).
So, the good PR's description is description how it was implemented instead of what is the problem about. Problem should be described in referenced issue.
Last but not least - hint for reviewers - it's good to read descriptions, as well.

[smola]

@kuba-- I really miss a good description in issues.

I agree that we also need far more effort in proper issue descriptions. But an issue description does not replace commit or pull request message. If the issue is properly written, then it can serve as much more detailed context as well as related discussion. The pull request/commit still benefits from a problem statement though.

the good PR's description is description how it was implemented

What and why still apply. What did you do to solve an issue and how? Why it was solved in a certain way and not in a different way? Even with well-defined issues, the exact scope of a given change, rationale of the "how", caveats, and so on, are not necessarily present in the issue in a concise way.

[bzz]

@smola @creachadair @kuba-- what may also be helpful here is to include some links to a good examples, illustrating the principles from the text. Would you be able to include 2-3 reference examples in here?

[creachadair]

@smola @creachadair @kuba-- what may also be helpful here is to include some links to a good examples, illustrating the principles from the text. Would you be able to include 2-3 reference examples in here?

On Kythe we used to link people to https://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html, and I think it's a good balance.

A lot of descriptions get into details about whether you should or shouldn't punctuate and capitalize, rules for tags, etc. I think most of that is a waste of time unless you plan to enforce the rules with presubmits: What we mainly want (I claim) is a low-friction set of simple guidelines that will help us do better.

Here's the canonical example from Tim's post:

Capitalized, short (50 chars or less) summary

More detailed explanatory text, if necessary.  Wrap it to about 72
characters or so.  In some contexts, the first line is treated as the
subject of an email and the rest of the text as the body.  The blank
line separating the summary from the body is critical (unless you omit
the body entirely); tools like rebase can get confused if you run the
two together.

Write your commit message in the imperative: "Fix bug" and not "Fixed bug"
or "Fixes bug."  This convention matches up with commit messages generated
by commands like git merge and git revert.

Further paragraphs come after blank lines.

- Bullet points are okay, too

- Typically a hyphen or asterisk is used for the bullet, followed by a
  single space, with blank lines in between, but conventions vary here

- Use a hanging indent

[smola]

@bzz Would you be able to include 2-3 reference examples in here?

I'm thinking about commits like this:

• Very informative: src-d/go-mysql-server@8f508ae
• Or far shorter, but still very relevant: src-d/proteus@511b531

These are just a couple of examples I found quickly. I'm not sure about setting "canonical" examples. Sometimes it's great to explain a lot of detail, sometimes it's overkill (but doesn't hurt either).

I didn't include conventions on formatting, which I think it would be better to discuss separately, since I predict much more bikeshedding there (e.g. capitalization yes or no, package prefixes yes or no), although there are some basics:

• one-line short summary, then blank line, then longer description
• if relevant, prefix with package name: (e.g. in Go)
• don't add a dot at the end of the short summary

[creachadair]

• don't add a dot at the end of the short summary

I wouldn't bother with this constraint unless it serves some tooling purpose. The other two seem good.

[smola]

Merging, deferred both the discussion on formatting and automation to another issue: #348