Non-opinionated code review guidelines
Tags: programming, codenewbie, review, softwaredevelopment
Published: 2023-04-06 14:26:41

---
title: Non-opinionated code review guidelines
description: There are only two objectives in a code review, i.e. get the best possible pull request, and improve skills of the fellow contributors. Easy enuf, except we need a set of ground rules!
published: true
date: 2022-09-02 05:53:17 UTC
tags: programming, codenewbie, review, softwaredevelopment
canonical_url: https://keepontruckin.hashnode.dev/non-opinionated-code-reviews
cover_image: https://user-images.githubusercontent.com/958227/233758735-dc03a104-3595-4cbe-8ca2-0147d660ef8a.jpeg?auto=compress
---

I'm not going to lie, <mark>opinions are like Arseholes, everyone has one</mark>. It would have been acceptable but the objectives of code reviews are simple. In general, not only is to get the best possible pull request, but also improving skills of the fellow contributors. Easy enuf, except <mark>we need a set of ground rules</mark>.

### Etiquette

The first thing we're going to need is a contributor etiquette. I like the idea of [Jante Law](https://en.wikipedia.org/wiki/Law_of_Jante#Definition). It gives everyone a very _Egalitarian_ feel since _you are not to think you're anyone special or you're better than everyone else_, as they teach in Scandinavian schools.

### Workflow

The next is the release engineering strategy. All we need is a version control tool and [git](https://git-scm.io/docs/) is the mainstream thingy for that. But [some of the branching strategies are pretty horrendous](https://atlassian.com/docs/git/gitflow/) so you have to <mark>keep the branch integrations to minimum</mark>.

![Using Git - Dilbert by Scott Adams](https://assets.amuniversal.com/ddf1fa20315201378d0e005056a9545d)

[Trunk-based development](https://trunkbaseddevelopment.com/) seems extraordinarily uncomplicated but there are more to consider.

- Branch policies for protecting the default branch.
- Naming conventions for branches.
- Pipelines for release engineering.

### Changesets

Some organizations often call this a _Pull request_ or a _Changelist_. And it has to be <mark>small-enuf to one self-contained change</mark>. Unfortunately, no playbook has a recipe for this so I'm going to have to let individual contributor decide the size of the pull request.

Once the pull request is ready to review, they have to describe it, specifically _what_ has been done and _why_ it was made as-is. This can be tricky, so a [conventional-based description](https://www.conventionalcommits.org/en/v1.0.0/#summary) seems like a great way of adding structure and meaning to it.

### Design and Health

![Its the holy grail of technology - Dilbert by Scott Adams](https://assets.amuniversal.com/268f57809f72012f2fe600163e41dd5b)

This covers a lot of ground and can be challenging enuf. Unfortunately, you can only boil so many oceans at the same time, so what does each reviewer should look for in a pull request? In general, _readability and maintainability_ would hint a few mistakes here and there, but there are more to review.

- Intended functionality and the design.
- Over-engineering and complexity.
- Clear naming and conformance to style guide.
- Parallel programming and thread safety.
- Sufficient exception handling and logging et al.

### Style Guide

Yes, a homegrown standards and conventions isn't such a great idea. The major issue is that it would demand an intense concentration to keep up with each tech stack without giving away the _[egalitarian culture](#heading-etiquette)_ completely since you would want to _Dictate-It-All_. And you do want contributors to maintain consistancy of the code. This may be difficult. I need a <mark>community-adept style guide</mark> to start with.

### Faster Reviews

A pull request won't get reviewed itself. And a contributor must intervene at some point. I can review shortly after it comes in. But that seems a little unproductive since it takes time to get back to _The Zone_ after been interrupted. So how fast should code reviews be? _One business day or 24 hours_ sounds reasonable but there is more to this.

- Professional courtesy when pointing out problems.
- Code analytics for obvious smelly codes, anti-patterns, conformance to standards, et al.

### Summary

![Too Dumb To Understand - Dilbert by Scott Adams](https://assets.amuniversal.com/dc424c1016c20134707c005056a9545d)

All we need is non-opinionated guidelines for all kinds of contributors, so in doing a code review, you should value for:

- <mark>Technical facts and data</mark> over opinions and personal preferences.
- <mark>Style guides</mark> over personal preferences.
- <mark>Software design principles</mark> over opinionated-design.
- <mark>Consistency</mark> of the code over personal preferences.
- <mark>Team velocity</mark> over personal velocity.

/KP

> **PS:** You can find more about the topic in my curated list of standards, practices, guides, and discussions on [my awesome list #33](https://github.com/kosalanuwan/keep-on-truckin/discussions/#readme).