Gatekeeping: Guide for gatekeeper
Note: This process is tailored to our specific needs and tools at makandra. While it will certainly not apply to all (especially larger teams), we think it is a helpful starting point.
In order to reduce the number of rejects we get from clients, we want to review all code written before it goes to the staging server.
If you're responsible for gatekeeping in a projects, this is what to do:
First, read the Gatekeeping for developers guide, then ask your developers to read it and follow it.
Role of the master branch
Code in the master branch has generally already passed the review process. Exceptions are fixes for customer rejects which will usually directly happen in master.
Every time a developer finishes a story, he will set it to "finished" on PT and open a merge request on GitLab. You will receive an e-mail.
Now review the code, either directly on GitLab, or by checking out the corresponding branch. You can assume that tests are green, but you need to confirm that
- everything requested in the PT story is implemented
- test coverage is good
- the code is maintainable
What to do if the code is okay
If the code is okay, you may either
- merge it into master yourself:
- Squash and merge into master
- Delete the branch with
git push origin :feature-branch-name
- Deploy to staging
- Set the corresponding PT story to "delivered"
- Close the merge request on GitLab
- If you like a more linear Git history, you may rebase the feature branch onto master first.
- or ask the developer to do so:
- add a note to the merge request, asking the developer to perform the merge, close the merge request and deliver the story
What to do if the code is not okay
If there are issues, you need to
- Add a note explaining the problem to the merge request on GitLab
- Reject the story on PT
Once the developer fixes the problem, he may decide himself
- if he wants it to be reviewed again:
- the developer will add a note in the merge request (you'll get an e-mail again)
- or if he does not think another review is necessary
- in this case the developer will himself have merged to master, closed the merge request and delivered the story
If you explicitly want to review the changes, please indicate so in your note.
Changes that do not need to be reviewed
As a good default, all non-trivial commits should be reviewed. As the gatekeeper you are free to make exceptions, e.g. for code written by yourself, for code written by very experienced colleagues or for trivial changes like fixing a typo.
If you want to make an exception, add a note to the story that the code should be commited into the master branch without going through a merge request.