Introduction
This article will touch upon scenarios that I have encountered while working at Hashnode where some code reviews helped us prevent future rework on the same issue by having a broader perspective.
Code Review 0
The Problem
Sometime back, one of our engineers, Kieran Roberts , stumbled upon a performance degradation when he was working on upgrading the navigation header behaviour for the blogs.
The issue wasn't strictly related to the header but the left sidebar. Whenever we closed the left sidebar on the blogs, the CPU spiked up to 100%.
This behaviour happened only in Chrome. Now that spike also affected our new header UX because it relied on doing some fancy listening to scroll events and style updates. The header navigation became janky. So this was a blocker that we wanted to get rid of first before shipping the header update.
As far as I remember, I think he found a workaround that prevented the CPU spike but did require making a tradeoff at that time which I can't exactly remember. When the PR for the same came to me for code review, I started digging into the GitHub issues around this behaviour in radix (a set of powerful UI primitives that we have been using lately to power a lot of UI on Hashnode) and ran into this issue thread:-
That is where I found out that the maintainer patched the behaviour a day ago that was making their Dialog radix-primitive to cause this CPU spike in Chrome and the patch was available in a release candidate for the same.
The Result
We updated our package version to the release candidate after confirming the same with the maintainer on that same thread and it did stop the CPU spikes.
We were honestly lucky to observe this issue at the right moment. The patch from the OSS was shipped at the right time for us because we ran into the issue just 2 days before it. For the most part, what this meant as a learning experience for me is that it helps to drop by the OSS repositories of the libraries/frameworks being used in your work to check if the bug/regression exists at the implementation end of that OSS code and not where it's used in your project.
Code Review 1
The Problem
This is much more recent, I think within the last month in fact, where a minor vulnerability was reported by a user.
I was again tasked with doing a code review for the PR which addressed a part of that vulnerability. Mostly, the PR dealt with addressing it more at UI end by modifying some bits. We were still unsure of where exactly at the backend API, one or the other check was missing which might have caused it and wanted to first ensure that at least from the UI end, it's not doable anymore. Post that, we would have investigated further.
As I was navigating through the PR, I thought just once to check the API implementation which existed in a separate repo. I found out that we already have the required checks in place and there was no reason that this vulnerability should exist. At that moment, I looked up when these checks landed in the code and it was again
Talk about coincidences.
The fix happened at the API level as part of a refactoring of one of our GraphQL mutations by Florian Fuchs . He was not aware of this issue and we were not aware that this was fixed just a day ago. But again, lucky sighting. To be double sure, we looped him in and got confirmation that indeed this was fixed by the refactor.
The Result
The confirmation closed the loop there itself. The timing was critical here because when Kieran could replicate the vulnerability, he made a UI-level patch as per it but when I code-reviewed it, the patch had already been done at the API level so there was this gap which needed to be bridged. What worked here well was that during the code review, just out of curiosity, I checked the API repo and didn't restrict myself to the scope of the original review.
Conclusion
It's weird that both these code reviews involved me and Kieran and had that one-day fix factor. I think these cases might be rare but since they have happened twice with me already, I thought of sharing these to maybe open up another perspective for other developers who are in the middle of making or reviewing that PR and can use it.
Thank you for your time. Do you have such experiences with code reviews? Feel free to drop a comment and share them. You might just save another developer a good amount of time.
