Git Standards
Git Refresher - https://www.atlassian.com/git/tutorials
Commit Standards
- The first line should always be 50 characters or less and that it should be followed by a blank line.
- Follow official Git cheat sheet - https://github.github.com/training-kit/downloads/github-git-cheat-sheet.pdf
- Answer the following questions:
- Why is this change necessary?
This question tells reviewers of your pull request what to expect in the commit, allowing them to more easily identify and point out unrelated
changes.
- How does it address the issue?
Describe, at a high level, what was done to affect change.
Introduce a red/black tree to increase search speed
orRemove <troublesome gem X>, which was causing <specific description of issue introduced by gem>
are good examples. If your change is obvious, you may be able to omit addressing this question. - What side effects does this change have? 1. This is the most important question to answer, as it can point out problems where you are making too many changes in one commit or branch. One or two bullet points for related changes may be okay, but five or six are likely indicators of a commit that is doing too many things. 2. Consider making including a link to the issue/story/card in the commit message a standard for your project. Full urls are more useful than issue numbers, as they are more permanent and avoid confusion over which issue tracker it references. - Never use the -m
/ --message= flag to git commit. It gives you a poor mindset right off the bat as you will feel that you have to fit your commit message into the terminal command, and makes the commit feel more like a one-off argument than a page in history:
git commit -m "Fix login bug"
A more useful commit message structure might be:
Some awesome title
Why:
* ...
This change addresses the need by:
* ...
Example -
Redirect user to the requested page after login
https://jira.com/path/to/relevant/issue
* Users were being redirected to the home page after login, which is less useful than redirecting to the page they had originally requested before being redirected to the login form.
* Store requested path in a session variable
* Redirect to the stored location after successfully logging in the user
Code Review Guidelines
Guidelines for what a code reviewer should look for, and how to give both subjective and objective feedback:
1. **The reviewer should identify errors that will cause an issue in production.** It’s a code review, after all, so the reviewer should identify missing semicolons, unending loops, or missing error handling. Reviewers aren’t responsible for finding *all* such errors (that’s still the responsibility of the author), but they should be on the lookout for obvious issues that will break the system if they’re are deployed into production. Such issues are a valid reason to block the pull request.
2. **The reviewer should verify that the stated goal of the code change aligns with the changes being made.** If the author submits a pull request that says they’re making changes to the networking code of a service, reviewers should expect that all of the changes are in and around the service’s networking code. This seems obvious, but it’s no secret that developers have a tendency to try to pack in multiple changes in such cases. This isn’t even necessarily a wrong practice, as long as the changes are mostly co-located. When you align a code change to its stated goal, however, you make it easier to determine if the pull request potentially submits any new bugs. Here, too, we agreed that failing to align the code change with its stated goal would justify blocking the pull request.
3. **The reviewer should verify that any changes align with the team’s coding standards.** I’ll cover this more in a bit, but as an example, if the team has decided that all variables must use camel case, and the reviewer finds a variable that does not use camel case, they should block the pull request.
4. **The reviewer should look for anything they personally disagree with.** This guideline addresses any comment which the first three rules do not cover. We *want* reviewers to give feedback, even if it’s not covered by the first three rules. We didn’t want our guidelines to suppress feedback, which is essential for how we learn from one another. Because these comments are clearly subjective, however, we agreed that they do not justify blocking the pull request.
Branching Model
At Indica Digital/Intelliflow Systems we use the following branching model -
- All of the branches will be merged in staging branch.
- Prefix before creating a new branch is important. Use the following -
- Feature branch - Used for specific feature work or improvements. Generally branch from, and merge back into, the development branch using pull requests. Format - feature/issue(ref Jira) E.g. feature/user_authentication b. Bugfix branch - Typically used to fix Release branches. Format - feature/bugfix(ref Jira) E.g. bugfix/login_failure c. Hotfix branch - Used to quickly fix a Production branch without interrupting changes in the development branch. ** should we merge this in staging or on testing? need to check atlassian ref. **
Branching Permissions
*Before we do this we need to make sure that branching model is setup.
Branch permissions help enforce specific workflows and prevent errors like a new team member deleting master. With branch permissions you can:
- Closely control which users or groups can write or merge to any branch.
- Create permissions for a specific branch type, or pattern. For example:
/PROJECT-*
to limit access to all branches with names likePROJECT-1234
. - Prevent users from force-pushing to a branch.
- Prevent a branch from being deleted.
Once you determine which branches a permission applies to, you then determine which actions to prevent, and optionally set exceptions to this permission. Prevent all changes Prevents pushes to the specified branch(es) and restricts creating new branches that match the branch(es) or pattern. Prevent deletion Prevents branch and tag deletion. Prevent rewriting history Prevents history rewrites on the specified branch(es) - for example by a force push or rebase. Prevent changes without a pull request Prevents pushing changes directly to the specified branch(es); changes are allowed only with a pull request.