If you're responsible for gatekeeping in a projects, here is a guide, what to do.
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.
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.
First, read the Gatekeeping for developers guide, then ask your developers to read it and follow it.
Role of the main/master branch
Code in the main branch has generally already passed the review process. Exceptions are fixes for customer rejects which will usually directly happen in main.
This branch can also be called master depending on the project.
Reviewing code
Every time a developer finishes an issue, they will set it to "Review" on Linear 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 issue 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 main yourself:
-
Squash and merge into main
-
Push
-
Delete the branch with
git push origin :feature-branch-name -
Set the corresponding issue to "Merged"
-
Deploy to staging
-
Set the corresponding issue to "On Staging"
-
Close the merge request on GitLab
-
If you like a more linear Git history, you may rebase the feature branch onto main 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 issue
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 issue on Linear
Once the developer fixes the problem, they may decide themselves
- if they want it to be reviewed again:
- the developer will add a note in the merge request (you'll get an e-mail again)
- or if they do not think another review is necessary
- in this case the developer will have merged it to main, closed the merge request and delivered the issue
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 issue that the code should be committed into the main branch without going through a merge request.
