DEV Community

Cover image for A Case History: Analysing Hasura Console's code review process
Stefano Magni
Stefano Magni

Posted on • Updated on

A Case History: Analysing Hasura Console's code review process

How a code review process can be speeded up? What are the practices we can follow to get PRs merged as soon as possible?

Introduction: I recently joined Hasura, and along with my new team we tried to identify the existing bottlenecks and why PRs stay open for such a long time. This aims to share our internal analysis that could be used to inspire other teams to do the same.

Related to the same topic, you can read my Support the Reviewers with detailed Pull Request descriptions article.

Photo by Lala Azizli on Unsplash


TL:DR

We, as PR authors, must strive to ease the reviewer’s life as much as possible.

The problems

  1. Reviewing a PR takes too much time.
  2. As a result, releasing features takes a lot of time.
  3. The problem is aggravated by the number of developers that recently joined the team.

Team members and review roles

Member Eng. seniority Hasura seniority Reviews code Reviews functionalities Time dedicated to reviews per day
N.B.S principal senior βœ… βœ… 2 to 5 hours (😱)
V.P.G. senior senior βœ… βœ… 2 to 2.5 hours
S.S. senior senior βœ… βœ… 2 hours
M.H. senior middle βœ… βœ… 1 hour
A.S.K. junior middle βœ… βœ… from 0.5 to 1 hour
V.C. junior middle βœ… βœ… 1.5 to 2 hours
S.M. senior junior βœ… ❌ 30 minutes
E.M. senior junior βœ… ❌ from 0 to 0.5 hours
N.I. senior junior βœ… ❌ from 0 to 0.5 hours
L.R. senior junior βœ… ❌ from 0 to 0.5 hours
D.C. senior junior βœ… ❌ ???

What do we dedicate the time spent on code reviews to?

Activity A.S.K. M.H. V.P.G. S.S. V.C.
Understanding what the feature is 10% 10-20% 10% 10-20% 10%
Reviewing the code 40% 40-50% 30% 20-40% 30-40%
Setting up the various environments 10% 10% 0-20% (if pro/cloud) 10-30% (if pro/cloud) 5-15%

Testing the functionalities

35% 30% 40-60% 40-70% (if pro/cloud) 50% (if Storybook is not used)

If we all spend 2 hours/day, we collectively

  1. spend 3 hours/day setting up the environment

  2. spend 10 hours/day reviewing the code (we can improve it through shared coding patterns and explaining the design choices we made)

  3. spend 11 hours/day testing the functionalities (the author can list what functionalities they already tested. Company-wide, they could consider creating a QA team)

What opening a PR requires

Activity Responsible Current status Future status
(pre) Describe the feature/bug The Product Owner ⚠️ (we must improve it) 🚧 We're working on it

(pre) Create coding patterns

The whole team ❌ ❌ Not in the roadmap
(pre) Groom the features The whole team ❌ 🚧 We're working on it
(pre) Define a testing strategy The whole team ❌ 🚧 We're working on it
(pre) Pair-programming the features The whole team ⚠️ (we must augment it) βœ… We can start right now
(pre) Reduce the scope of a feature/bug The whole team ⚠️ (we must improve it) 🚧 We're working on it
(pre) Add unit/integration tests PR's authors ⚠️ (we do not write a lot of tests) 🚧 We're working on it
(pre) Add E2E tests PR's author ❌ (we do not write E2E tests at all) 🚧 We're working on it
(pre) Tidy up the commits PR's author ⚠️ (mostly by squashing them) βœ… We can start right now
Describe the feature/bug PR's author ⚠️ (some of the PRs have short descriptions) βœ… We can improve right now
Describe how to test/reproduce it PR's author ⚠️ (sometimes it is not described at all) βœ… We can improve right now
Describe the edge cases PR's author ⚠️ (some of the PRs have short descriptions) βœ… We can improve right now
Describe the chosen solutions and motivations PR's author ⚠️ (some of the PRs have short descriptions) βœ… We can improve right now
Describe the discarded solutions and motivations PR's author ❌ βœ… We can improve right now
Describe the codebase limitations PR's author ❌ βœ… We can improve right now
Add screenshots/videos PR's author ⚠️ (not always done) βœ… We can improve right now
Add screenshots of the original graphic layouts PR’s author ❌ βœ… We can improve right now
Correctly fill up the PR template PR's author ⚠️ (we must improve it) βœ… We can improve right now
Comment out when starting the review PR's reviewer βœ… βœ… We can improve right now
Review the code PR's reviewer βœ… βœ… We can improve right now
Speeding setting up the Console in all modes The whole team ❌ 🚧 We're working on it
Review the graphic layouts alignment PR's reviewer ⚠️ (not always done) βœ… We can improve right now
Review the functionalities PR's reviewer ⚠️ (sometimes require double check, sometimes not done properly on all the versions of the Console) βœ… We can improve right now
Describe what has been tested in the review PR's reviewer ❌ βœ… We can improve right now

What we should do

  1. Anticipate and share the problems as soon as possible (through the grooming sessions)
  2. Share what we will do, what we are doing, and what we did with other team members
  3. Move part of the time dedicated to the code review to organize and describe the PR, by asking ourselves
    1. Have I described the feature enough? Is there anything I could do to get it more understandable?
    2. Are my intentions clear enough?
    3. Does my code reflect the intentions?
    4. Have I tested the feature the best possible way? Have I described what I tested?
  4. Spread the knowledge of the product and its edge cases to the new team members

In one word: tell!

Top comments (0)