Add branching model team agreement doc #18
Conversation
| - Reviews can be done by the first person who gets to it. If the code needs explaining, request the author to walk you through it | ||
| - Follow peer review best practices by suggesting opportunities to improve code during peer review, merging as soon as the code is better than the code in the target branch and release ready | ||
| - Treat any opportunity for improvement feedback identified during peer review but not implemented in the PR where it was raised as technical debt worthy of a new issue referencing the PR where the comments first came up | ||
| - Branch naming policy is as follows: dm-issue#-brief-description |
There was a problem hiding this comment.
Is the dm- prefix performing some meaningful function? (per the example of this branch being dm-14-branching-model-agreement) Or could we do with a more succinct issue#-brief-description?
There was a problem hiding this comment.
In the upstream there's a pattern (maybe team agreement) of beginning each branch with the JIRA ticket number. Perhaps we continued that practice in this repo? A team agreement on consistent naming would be helpful...though there's often a one-to-many relation between an issue and the PRs that close it. Perhaps we could consider including the ticket number we're closing in the description section of the commit that substantially delivers the feature? For single-commit pull requests (or draft PRs that begin after pushing the very first commit) the title and description of the commit message are auto populated into the github PR title and description.
There was a problem hiding this comment.
The dm- prefix is something we're doing in the upstream, so we decided to keep it the same here
There was a problem hiding this comment.
| - Branch naming policy is as follows: dm-issue#-brief-description | |
| - Branch naming policy is as follows: brief-description |
There was a problem hiding this comment.
Perhaps there's a risk that by keeping it the same as upstream we will confuse references to JIRA ticket numbers (the DM-xx form) with the Github Issues of this fork? In standup @BCerki asked if the suggestion above was a typo. The option of beginning branches with the issue number they relate to issue#-brief-description is another possible team agreement, and the impact of that decision would be to create a mechanism whereby we avoid making pull requests that don't correspond to tracked issues. As we were discussing in standup, sometimes a quick refactor is best enabled by directly sending a pull request without creating an issue–sometimes trivial refactors unblock our feature tickets. Additionally, I'd note that our branches are a transitory artifact–per suggestion above migrating the issue linkage to commit messages themselves would make this ephemeral linkage permanent and immutable.
There was a problem hiding this comment.
Marked this unresolved until we fully migrate discussion to #21 so it shows when people follow link from that issue.
| ## Guardrails | ||
| - The default branch for this repo is `main` | ||
| - all commits must be made to a non-protected branch and submitted via a pull request before they can be merged into `main`. (There can be no direct commits made to `main`.) | ||
| - Pull requests require at least 1 approval before the can be merged to `main`. Any commits made to an approved branch will dismiss the approval. (If you commit new code to a branch after it gets approved but before you merge it, you have to get it approved again.) |
There was a problem hiding this comment.
| - Pull requests require at least 1 approval before the can be merged to `main`. Any commits made to an approved branch will dismiss the approval. (If you commit new code to a branch after it gets approved but before you merge it, you have to get it approved again.) | |
| - Pull requests require at least 1 approval before they can be merged to `main`. Any commits made to an approved branch will dismiss the approval. (If you commit new code to a branch after it gets approved but before you merge it, you have to get it approved again.) |
|
|
||
| ## Guardrails | ||
| - The default branch for this repo is `main` | ||
| - all commits must be made to a non-protected branch and submitted via a pull request before they can be merged into `main`. (There can be no direct commits made to `main`.) |
There was a problem hiding this comment.
| - all commits must be made to a non-protected branch and submitted via a pull request before they can be merged into `main`. (There can be no direct commits made to `main`.) | |
| - All commits must be made to a non-protected branch and submitted via a pull request before they can be merged into `main`. (There can be no direct commits made to `main`.) |
| - Pull requests require at least 1 approval before the can be merged to `main`. Any commits made to an approved branch will dismiss the approval. (If you commit new code to a branch after it gets approved but before you merge it, you have to get it approved again.) | ||
| - Pull requests must pass CI checks in order to be merged to `main` | ||
| - Branches are automatically deleted when merged | ||
| - Squash merge has been disabled |
There was a problem hiding this comment.
| - Squash merge has been disabled | |
| - Squash merge has been disabled to increase efficiency of `git bisect` |
| - all commits must be made to a non-protected branch and submitted via a pull request before they can be merged into `main`. (There can be no direct commits made to `main`.) | ||
| - Pull requests require at least 1 approval before the can be merged to `main`. Any commits made to an approved branch will dismiss the approval. (If you commit new code to a branch after it gets approved but before you merge it, you have to get it approved again.) | ||
| - Pull requests must pass CI checks in order to be merged to `main` | ||
| - Branches are automatically deleted when merged |
There was a problem hiding this comment.
| - Branches are automatically deleted when merged | |
| - Branches are automatically deleted on Github when a PR is merged. Each teammate should set `git config fetch.prune true` to mirror this behaviour to their local workstation. |
kriscooke
force-pushed
the
dm-14-branching-model-agreement
branch
from
f9b5d60
to
790d355
Compare
|
Kudos, SonarCloud Quality Gate passed!
|
Starting point for the agreement discussed by the team.