Guidelines for team development using Git / GitHub
We all use Git and GitHub for development, but if there's a subtle difference in understanding between team members on how to use them, it can hinder progress. In order to prevent that from happening, we have established guidelines.
This is an English version of this article.
Situation
In the CX Business Division, we work on development projects for clients and development of our own services, and (we think) these guidelines are useful for this.
We have possibilities that team members may change for various reasons, and it is meant to help new members get started. Therefore, the main focus is on the ability to perform effective skill transfer without reducing development efficiency as much as possible.
Guidelines
Here is the all of the guidelines.
It is divided into three sections.
for commit
avoid committing tangled changes
A tangled change is a change that has more than one intention or purpose. It's a word in software engineering, but I use it because it says what I want to say in two words. For an exact definition, you may want to read the following paper.
Since these papers are written by software engineering researchers, they also include an argument for a disadvantage for them, which is that they are difficult to analyze code bases by tangled changes. The disadvantage for developers is simply that the intent of the commit is obscured and cannot be reverted.
That's why I'm asking my team members to get rid of tangled commit in my project.
commit changes to build and pass unit-tests
This indicates to commit codes to pass unit tests always.
This also implies that you should write the tests at time to commit. but I'm referring to testing as a fixture for design, not a fundamentalist one that says you should write the test first, as in TDD.
make a lot of small commits rather than a huge commit
If you've ever done a code review, you'll know this, but big pull requests don't get reviewed anyway. In addition to the essential tasks such as learning the prerequisites, confirming underlying changes, and identifying changes to the main plot, it is quite a heavy workload when you include other tasks such as pointing out how to write and considering the direction of the changes.
In order to prevent this kind of code review from becoming a heavy workload, we want people to make small changes to accomplish our goals instead of committing one big change.
Roughly speaking, I think the review will go forward with no problem if you do the following.
- (if tests doesn't exist) writing tests
- refactoring for better understanding
- making underlying changes
- making changes in the main plot
- consolidation through refactoring
No. 0 and 1 might be different Pull Requests.
for Pull Request
These are easy to implement with Pull Request Template to assist with the input.
write related issue number
Often, the issue will speak for the premise and purpose of the Pull Request, which can be a clue to the reviewer's understanding.
There is also a function to close the specified issue when the pull request is merged with a specific keyword, so take advantage of it.
show screenshots before / after the changes when you make any changes about UI
UI changes may be difficult to determine from the code whether they are appropriate or not.
To reduce the burden on the reviewer, it is a good idea to post a screenshot before and after the change. If you're making changes to a moving part, it's a good idea to include a video.
merge guideline
merging PRs needs 1 or more approvements
There's a GitHub feature that protects branches, and there's a setting called [Require pull request reviews before merging] (https://help.github.com/en/github/administering-a-repository/enabling-required-reviews-for-pull-requests).
Let's enable this and make the review mandatory. Reviews are a great learning opportunity as well as a way to deepen the team's understanding of the code and eliminate depending on individual skills. Take a positive view of conducting a review and use it effectively.
merge PRs with creating a merge commit
GitHub gives us three options for merging pull requests.
- Create a merge commit
- Squash and merge
- Rebase and merge
The guideline is to use No. 1. "Squash" is the process of merging the commits included in a Pull Request into a single commit. This should be avoided because it will be difficult to read the details of the change later. Also, since "Rebase" can cause conflicts, you should merge the merging target at your fingertips and push it back if necessary.
PRs should be merged by the owner, because the PR owner knows the risk better than other people
The point is that the author knows the scope of the pull request best, so merging it is less likely to break by the author. Also, it feels good to merge the Pull Requests they've created.
Summary
These guidelines have been in use for quite a long time, and I have asked developers to do so each time. Everyone is reasonably convinced that they can put it into practice.
If you have your own guidelines, please let me know. I'd like to know how other teams do it.
