How to protect GitHub projects from non-reviewed code and force code review culture
Tags: engineeringmonday, github, codereview, productivity
Published: 2021-08-31 07:39:24

In this blog post I will show you a way of protecting GitHub repositories from random pushes of non-reviewed code or pushes to the `master`/`main` branch. I’m a strong believer in deploying features via pull requests with code review. I will not get into the question if the branch and pull request workflow is good or bad, I have my opinion which is that pull requests and code reviews are a must for a team that wants to learn, so if you don't agree this article is not for you because it forces all team members to use it.

Code review is a practice that depends on the culture. A culture with no ego, with a great wish for continuous learning, sharing and teamwork. Besides learning code, code reviews will improve your communication soft skills as you will need to be clear and professional without being harsh which will also show you if you are a good mentor or not.

This article will focus on 3 steps to achieve pull request with code review practice:

- Workflow in theory 
- Setting up your project
- Creating pull request template

### Workflow in theory

1.On GitHub create a new branch from `master` and use standard naming convention for branches:

- `feature/name-of-the-feature`
- `fix/name-of-the-fix`

Try to do this for every `feature`/`fix` to avoid creating very large pull requests which will be very time consuming for reviewers.

2.After you finish working, commit and push your code to your `feature`/`fix` branch and create pull request to merge this branch to `master` branch.

3.Assign someone for code review. The goal here is to learn from each other and to make sure all standards are met and code style is respected, and of course to make sure code does not have any bugs. 

4.If there are any questions/suggestions/fixes/changes requested from the person which is conducting code review, that person will request changes on GitHub with clear comments and the process starts again. All comments need to be resolved before the reviewer can accept the pull request, either changes or only answering the questions.

5.After successful code review, the branch will be merged into the `master` branch and the `feature`/`fix` branch will be automatically deleted.

### Setting up your project


#### 1. Create `CODEOWNERS` file


Code owners file defines individuals or teams that are responsible for code in a repository. Code owners are automatically requested for review when someone opens a pull request that modifies code that they own. To use a `CODEOWNERS` file, create a new file called `CODEOWNERS` in the `root`, `docs/`, or `.github/` directory of the repository, in the branch where you'd like to add the code owners. I’m a simple guy so I always put everything in the `root`. You can assign different code owners for different branches. In practice, no one could approve pull requests without code owners approval. This will protect developers from trying to merge pull requests that no one approved.

Example `CODEOWNERS` file:

```
* @marioloncarek
```

#### 2. Manage user roles


On GitHub repository go to the `Settings` tab and then choose `Manage access` from the left menu. Here you can define which users can access your repository and what are their roles. Always have at least one administrator, and all other developers can have write permissions. In practice, administrators could override everything from this article and use their privileges to make changes on master branch or force merge without review. This could be helpful for hotfixes.

![Alt Text](https://dev-to-uploads.s3.amazonaws.com/uploads/articles/swdnah795f1ije4m0ybd.png)

#### 3. Configure branch protection settings

On GitHub repository go to the `Settings` tab and then choose `Branches` from the left menu. Under `Branch protection rules` click `Add rule`. 

![Alt Text](https://dev-to-uploads.s3.amazonaws.com/uploads/articles/8oaw6a95dh7c8c7z3rou.png)

This will open branch protection configuration. Under `Branch name pattern` write your main branch name (probably `master`) or any other branch you wish to protect. Configure all options according to this image:

![Alt Text](https://dev-to-uploads.s3.amazonaws.com/uploads/articles/1v8u1bjmg093g7o5hyvi.jpg)

This configuration will:

- require pull request reviews before merging 
- require review from code owners
- restrict who can dismiss pull request reviews
- require status checks to pass before merging
- require branches to be up to date before merging
- require conversation resolution before merging
- restrict who can push to matching branches
- disallow force pushes for all users with push access
- disallow users with push access to delete matching branches

### Creating `pull_request_template.md` file

When you add a pull request template to your repository, project contributors will automatically see the template's contents in the pull request body.

To make your pull request template visible in the repository's root directory, name the pull request template `pull_request_template.md` and put it in the `root` of the repository.

Now when a contributor creates a new pull request they will see the template which will make pull request more standardised and contributors can be reminded about stuff that is important for the project (like standards, code style, build flows etc etc) with checklists.

Example `pull_request_template.md` file:

```
## Description

Please include a summary of the change or which issue is fixed.

## Type of change

- [ ] New feature (non-breaking change which adds functionality)
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)

## Area of change

- [ ] Frontend
- [ ] Backend

## General checklist:

- [ ] My code follows the style guidelines of this project
- [ ] I ran `npm run format`/`yarn format` before commit
- [ ] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation where needed
- [ ] My changes generate no new warnings
- [ ] I have checked my code and corrected any misspellings
- [ ] I have updated `master` and merged to my branch before submitting pull request
- [ ] My pull request generate no conflicts with `master` branch
- [ ] I requested code review from other team members

## Frontend checklist:

- [ ] I followed guidelines for `HTML`/`LIQUID`, `SCSS`, `JAVASCRIPT` from readme
- [ ] My `Javascript` generate no new console errors  
- [ ] I tested my code cross browsers
- [ ] My slice is pixel perfect for both desktop and mobile according to design
- [ ] I conducted basic QA to assure all features are working
- [ ] I tested responsive for mobile and tablet resolutions

## Backend checklist:

- [ ] I tested admin by manually adding content from zero
- [ ] I followed guidelines for naming admin fields
- [ ] I created easy to use admin experience which is self-explanatory
- [ ] I added description to admin fields in hard-to-understand areas
- [ ] I followed guidelines for naming `php`/`liquid` variables
- [ ] I conducted basic QA to assure all features are working
```

### Conclusion

These 3 setup steps will create strong protection of repositories against pushes of non-reviewed code or direct pushes to the master/main branch. It will force the team to apply the rules and maintain the code review culture.

It’s not always easy to enforce people to follow the rules, but with suggestions given throughout the post, you can automate them quickly, to make sure that everyone follows them.

What do you think about rules mentioned above? Do you have other advices to improve your team’s workflow?