perlhack: We use p.r's, not issue tracker; clarify commit msgs#24134
perlhack: We use p.r's, not issue tracker; clarify commit msgs#24134khwilliamson wants to merge 2 commits intoPerl:bleadfrom
Conversation
This splits the explanation of a commit message into 1) title and 2) body, adding significant details.
khwilliamson
force-pushed
the
commit_msgs
branch
from
8fdb6a3 to
4609cac
Compare
|
I do not approve of this pull request. My initial reaction is: It doesn't practice what it preaches. Its subject line largely consists of "We use p.r's, not issue tracker" -- but the pull request itself only mentions 'issue tracker' in the deletion of two lines from the existing documentation. More generally, what constitutes a good commit message is something that is always going to be up for argument. The content of this pull request would make a great post on a blog, but it's not something I think we need to codify in our official documentation at this time. |
|
Quoth @jkeenan
That sounds like a very good reason to document the standards. |
| the body can be omitted if the title adequately describes the change. | ||
| In such cases, you can use a git shortcut to create the commit: | ||
|
|
||
| % git -a -m'perlhack: Fix spelling error' |
There was a problem hiding this comment.
It might also be a good idea to not recommend -a, since it attracts cruft in commits...
| % git -a -m'Use getentropy() for seeding PRNG in Perl_seed()' | ||
|
|
||
| In a few cases, a file: | ||
|
|
||
| % git -a -m'embed.fnc: Add pointer assertions for refcounted functions' | ||
|
|
||
| Sometimes, a module: | ||
|
|
||
| % git -a -m'Devel::PPPort: Add support for utf8_to_uv' | ||
|
|
||
| And rarely, entirely omitted: | ||
|
|
||
| % git -a -m'Add feature "class"' |
There was a problem hiding this comment.
Several missing "commit' from "git commit"
| % git -a -m'"Where": Add/clarify comments" | ||
| % git -a -m'"Where": Use more mnemonic variable name" | ||
| % git -a -m'"Where": Move ARGS_ASSERT to top of function" |
| =item * Why | ||
|
|
||
| Your commit message should describe why the change you are making is | ||
| The message body should describe why the change you are making is |
There was a problem hiding this comment.
It might be worth mentioning the "Fixes #..." magic, which serves a few purposes:
- it mentions the ticket number (part of "Why")
- it closes the ticket once the PR is merged
- someone who views the ticket sees the link to the PR too
There was a problem hiding this comment.
It's also worth mentioning that only including the ticket reference / number / url and nothing else can result in not being able to find issues in the far future when the team/queue/ticket system goes away.
|
Couple of quick ones:
|
5 participants
This adds extensive discussion about what to put in a commit message, and especially the title.