Add guidelines about commit messages and merging/squashing commits #5691
Conversation
|
@open-telemetry/collector-contrib-approvers please also review. |
tigrannajaryan
added
the
Skip Changelog
Codecov Report
@@ Coverage Diff @@
## main #5691 +/- ##
=======================================
Coverage 91.53% 91.53%
=======================================
Files 192 192
Lines 11398 11398
=======================================
Hits 10433 10433
Misses 770 770
Partials 195 195 Continue to review full report at Codecov.
|
Our commit messages often are quite lacking. A typical approach is writing single line commit
messages and pushing multiple such commits to a PR. When commits are squashed the commit
messages are concatenated, and we end up with useless commit messages.
Here is an example of a commit message body (I am omitting the first line and the commit hash
to avoid blaming anyone in particular - we are all guilty):
```
* add docstring
* fix typo
* fix impi
* apply review feedback
```
This is a useless commit message, in fact, it is worse than an empty one.
I tried to keep the added paragraph in the CONTRIBUTING.md short. If you think more details and
convincing is necessary I can make it more elaborate. Note that the text I added doesn't just
call for good commit messages but has some hints about the commit and PR creation, squashing and
merging processes that make it easier to end up with better commit messages.
tigrannajaryan
force-pushed
the
feature/tigran/commit-messages
branch
from
1805092
to
758ee77
Compare
|
Let's see what commit message we end up with when this is merged :-) |
| Use descriptive commit messages. Here are [some recommendations](https://cbea.ms/git-commit/) | ||
| on how to write good commit messages. | ||
| When creating PRs GitHub will automatically copy commit messages into the PR description, | ||
| so it is a useful habit to write good commit messages before the PR is created. | ||
| Also, unless you actually want to tell a story with multiple commits make sure to squash | ||
| into a single commit before creating the PR. |
There was a problem hiding this comment.
[Controversial opinion] we should never need more than one commit, if you need more commits it means you actually need more PRs.
There was a problem hiding this comment.
I don't disagree, especially because we force squashing the commits when merging, so multiple commits are only useful as a tool to aid reviewing (i.e. tell a story about how the PR was created). I wanted to leave this option open, although I don't think we use it much (or ever).
|
@open-telemetry/collector-approvers any other comments or good to merge? |
Our commit messages often are quite lacking. A typical approach is writing single line commit
messages and pushing multiple such commits to a PR. When commits are squashed the commit
messages are concatenated, and we end up with useless commit messages.
Here is an example of a commit message body (I am omitting the first line and the commit hash
to avoid blaming anyone in particular - we are all guilty):
This is a useless commit message, in fact, it is worse than an empty one.
I tried to keep the added paragraph in the CONTRIBUTING.md short. If you think more details and
convincing is necessary I can make it more elaborate. Note that the text I added doesn't just
call for good commit messages but has some hints about the commit and PR creation, squashing and
merging processes that make it easier to end up with better commit messages.