Talk:Gerrit/Commit message guidelines/Archive 2

Previous discussion was archived at Talk:Gerrit/Commit message guidelines/Archive 1 on 22 March 2019.

Done via this edit by Timo.

One of the key things I think these guidelines miss is the need for titles to uniquely identify what is changing, distinct from other potential commits.

foo: Fix function documentation

… is pretty unhelpful, whereas:

foo: Document function's return types with '@return'

foo: Update documentation to use '@chainable' over '@return self'

foo#bar: Remove `@chainable claim`, this method might return null

… all give much more information (and yet could all be represented by the first title, as I've seen it happen).

Maybe after "Keep in mind that this will be in the repository forever." we could add "Ensure your title briefly but uniquely explains what you have changed." or similar?

(My casual term for this requirement is commit message "passing the Timo test".)

Thanks for added this meanwhile with diff 4905068 in 2021. I apparently copyedited that a few months later. Not remembering either edit, I also added an example just now upon reading this old thread!

Your subject line should be short, clearly state what your commit is changing.It should not be possible to use the same subject line for two commits that do different things. Consider "Fix FooBar docs to use @chainable" instead of "Fix a function doc".

As part of the guidelines for the body we suggest keeping lines with a max of 100 characters. That doesn't work anymore with the new gerrit UI. I suggest we change it to 65-70 max (unless we can change the width of that box)

The industry standard seems to be 72 characters for the body. Perhaps Gerrit is expecting that.

[Posting here because @TK-999 boldly changed the guidance from 100 to 72, and I reverted.]

I slightly disagree; the length of 100 chars works well enough. Happy to go with the flow if there's consensus, of course.

Also, given we're migrating off gerrit, I think we shouldn't worry too much about current UX issues there, assuming this guidance will outlast it.

I agree, no need to discuss or change this now.

I’ve slightly updated the text (mainly because I disliked the guidance to manually wrap the message), and added a mention of the customary 72 limit, while leaving 100 as the maximum. permalink

Is there any reason we don't allow full URLs for bug ids? i.e. https://phabricator.wikimedia.org/T321134 rather than just T321134?

The Gerrit (and now GitLab) integrations with Phabricator do not expect or support full URLs. We could look into changing that, but I would say that's where the convention comes from.

There is I believe a long standing tradition in software development to refer to tickets and issues by ID.

Eg. "Fixes #123" in Trac, "FOO-123" in Jira, "bug 123" in Bugzilla, "D123" at Facebook/Diffussion, "Issue 123" in Monorail/Chromium/Google. And more recently on GitHub with "#123" and "gh-123" to mention, resolve, or close issues and pull requests. Phabricator does this as well between tasks.

In MediaWiki, we adopted this during the time we used Bugzilla. You also see it in code to refer to tasks more succinctly. I suppose we value visual brevity, and it does seem appealing to explain (and support/maintain) only one way to write/parse something.

Each bug ID on its own line:

Bug: T299087
Bug: T299088

I tried comma-separating them, like "Bug: T299087, T299088" but that gives me the following error:

The following errors were found:

12:12:56 Line 3: Bug: value must be a single phabricator task ID

"put each Bug: T12345 reference on its own line at the bottom."

Bug: T299087
Bug: T299088

The middle of the current "Good example" of a commit message is currently just nonsense placeholder text. It would be better if this were realistic text, given that this is supposed to help people learn what to write.

I don't understand. The example shows the title with modifier, a bunch of prose, and some references for future work, exactly as required. Inventing "nicer" prose seems very low value?

What should people write in that "bunch of prose" in the middle? How detailed should it be? Should it restate what's in the title? Should it mention which files have changed? The current example answers none of those questions.