Code Reviews

All code at Talis must go through peer review before it can be merged into the main branch. This can be done by raising a pull request (henceforth “PR”) on the relevant GitHub repository and asking your colleagues for input.

30% vs 90%

Early feedback on an outline plan for your code is great. We follow the 30% feedback rule¹ which allows developers to get early feedback on their approach before dotting the i’s and crossing the t’s. Simply label your pull request with a “30%” tag to get this kind of feedback.

Later, when you are closer to merge, and need a pedant, re-label your pull with a “90%” tag to get feedback on the details.

You’ll find that smaller changes (one liners or very small features) will skip the 30% stage and go right to requesting 90% feedback.

Authors may raise PRs prior to requesting a review - to track their own changes or organise their thoughts. If a PR has neither a “30%” nor “90%” tag, please do not comment unless the author asks you to.

Preparing for review

If you are asking for a review, there are some steps you should follow before passing your PR to a reviewer.

First, please ensure you have sufficient description on your PR. You wrote the code, and you understand it best, but that also means you’re susceptible to the curse of knowledge². Put yourselves in the reviewers’ position, and bring them along the journey to understanding.

Second, please perform a self-review before asking others to review your code. This is to chiefly for the above reason - to ensure you understand the thing that you are reviewing, and have explained the salient details to assist with the review. However, this also serves to check that you’ve followed any conventions and rules that apply - it’s far better to spot these yourself and fix them before they reach a reviewer.

Third, it’s always a good idea to add comments to draw attention to areas that need focus. The majority of a PR may just be things needed to get something demonstrable; but the bit that you’re trying to demonstrate deserves more critique and attention. Guide your reviewers’ attention to where it is needed most. You might also use comments to guide reviewers to ignore certain things - e.g. a shortcut taken for 30% demonstration that will be rectified by 90%.

Finally, please keep your PRs on-topic.

In summary: show empathy for your reviewers.

How do I perform a Code Review?

Don’t always rely on the diff - depending on the size of the changes in the pull request, it might be a good idea to check out the branch locally. This gives you a number of benefits:

You can open up your IDE and see the code as it was written. A variable may have been renamed - you can search in your IDE for the old values.
You can spin up the app and take it for a test drive. The code might look ok, but the UI may look terrible and spam the console with errors, APIs might be hard to use, and it might run like a dog.
You can run the local build (including tests) and see any potential breakages.

When can I hit merge?

There are no hard and fast rules about when a PR can be merged, except that you have to have had a round of feedback from a peer at 90% level, and responded to all raised issues either by committing further work or resolving any discussion in the PR itself. You may be merging into a project which another team is the owner for, this is encouraged! In doing so, you should make sure that a least one approval is from a member of that team.

We use Github branch rules to ensure that PRs must receive at least one code review approval, and have passed all required automated checks.