Add git workflow guidelines #2688
Conversation
github-actions
bot
temporarily deployed
to
branch-team-git-workflow
doc/contributing/docs/git.rst
Outdated
| * Use the imperative mood. | ||
| * Start with a capital letter, don't add ending punctuation. | ||
| * Try to stick to 50 characters or so. |
There was a problem hiding this comment.
AFAIU it is about commit message titles?
doc/contributing/docs/git.rst
Outdated
| Commit messages | ||
| --------------- | ||
|
|
||
| * In your commit message, convey the nature of the change and possibly the reason why it was made. |
There was a problem hiding this comment.
From my experience (and our commit message guidelines supports it) the reason is the most important thing. It is what you often can't deduce from the changes.
There was a problem hiding this comment.
It may be so for code, but it's a bit different for docs. Text often speaks for itself. Cases when a commit message needs more than one line are extremely rare in my practice.
github-actions
bot
temporarily deployed
to
branch-team-git-workflow
patiencedaur
force-pushed
the
patiencedaur/team-git-workflow
branch
from
c6d5a0c to
582fb7d
Compare
github-actions
bot
temporarily deployed
to
branch-team-git-workflow
|
I propose adding https://xkcd.com/1296/ as an illustration ) |
I appreciate the idea, but it may be unfitting since we're leaning towards formal language in our docs. |
github-actions
bot
temporarily deployed
to
branch-team-git-workflow
doc/contributing/docs/git.rst
Outdated
| * Resolves #1234 | ||
| * Fixes #1234 | ||
|
|
||
| If your PR closes more than one issue, add such a line for each issue. |
doc/contributing/docs/git.rst
Outdated
| Commit messages | ||
| --------------- | ||
|
|
||
| * One-line commit messages are sufficient for documentation changes. Try keeping it 50 characters or less. |
There was a problem hiding this comment.
There can be a second line with "Resolves #...". In fact, a commit is often made before opening a PR.
There was a problem hiding this comment.
When you squash commits at merge, the resulting message will be a sum of commit messages. So the first commit should have the "resolves" string. Otherwise, there's a risk that this line won't be in the resulting commit.
There was a problem hiding this comment.
So, the best practice is to write a good commit message first, and then to open a PR
There was a problem hiding this comment.
In fact, a commit is often made before opening a PR.
Do you mean this process: First you implement changes locally, then you push the branch with the commit, which prompts to create the PR?
the first commit should have the "resolves" string
You mean that the first commit message in the branch must have a second line with the "Resolves" string? Currently we don't do that, I think. But we do include the "resolves" string in the PR description when creating the PR. Do you think we really need to introduce this practice?
| Merging | ||
| ------- | ||
|
|
||
| Merge when your document is ready and good enough. |
There was a problem hiding this comment.
- Always squash commits
- Make sure the commit message mentions all relevant issues with "resolves" or "fixes"
- Make sure you've attributed all participants with Co-authored-by
github-actions
bot
temporarily deployed
to
branch-team-git-workflow
github-actions
bot
temporarily deployed
to
branch-team-git-workflow
github-actions
bot
temporarily deployed
to
branch-team-git-workflow
github-actions
bot
temporarily deployed
to
branch-team-git-workflow
Fixes #1884