Two tips for improving your Code Reviews

manuartero

Manuel Artero Anguita 🟨

Posted on July 24, 2023

Two tips for improving your Code Reviews

You know the deal; Code Reviews should focus on the big-picture. The relevant-deep issues related to software architecture while avoiding the code style, formatting, and subjective opinions...

...in theory, that is

I mean, the important stuff should take precedence. Here are a few things that come to mind:

  • Consider breaking changes: will these changes cause chaos in the current system?
  • Keep a consistent API: will these changes cause chaos in external systems?
  • Does it solve the thing? (is it correct?)
  • Security issues?
  • Concurrency issues?
  • Double check when adding new dependencies
  • Do you need to update the documentation?
  • Do you need to update the test coverage?
  • ...

I am not saying the opposite.


But subjective things count too.

A roadside cafe close to a sunny road. Anime style. Generated by leonardo.ai

At least while other people is going to read and maintain the code base (not sure what's to happen within 10 years 🤖), let's not minimize or ignore their opinions regarding readability.

Most of the time there is no objectively better naming or a unquestionable better way of organizing a module.
What's crystal clear to one coder, it's a Lovecraft horror to another 🦑.

The real problem is, this sentence:

"this is more/less readable"

is a matter of perspective.

It's not that readability isn't important; it's just hard to measure "how readable" a piece of code is.


Here is a piece of advice - a tip if you want - that works for me:

  1. ❌ Avoid pointing out just what you don't like;
  2. ✅ DO write down the alternative you'd prefer.

Since all of this is subjective and revolves around reading preferences, please make sure to articulate your choice thoroughly. Reading actual code - instead of imagining the alternative - may lead to better comprenhension

Ah! and the closer to real code, the better (I mean, try to avoid asdasdads-ing).
Write actual code.


On GitHub, there is a wonderful «make suggest» button:

Screenshot from GitHub's UI where there is a make-suggestion button on the PRs

This is going to wrap your co-worker's line with suggestion special mark (GitHub flavored markdown).

This is a dummy example of a rename request; let's say I'd prefer to name this particular method showError() instead of setApiError(). And take the chance to rephrase the typing declaration.

Both are minor, subjective change requests.

Neither the rename, neither the type declaration may be tagged as "important stuff".

Screenshot from GitHub's UI where we are using the make suggestion for requesting a method rename

My point is that offering the actual alternative makes easier to evaluate if this minor change is acceptable.

The idea is: if both programmers agree on this minor thing... why not changing it?

Screenshot from GitHub's UI where we can see how the suggestion is finally shown

...otherwise, do NOT start a renaming discussion. Again, this is not on the "important stuff" list. Do not spend more time on this matter.

You proposed a quick suggestion, move on.


If you're not coding on GitHub (or just that the suggestion thing isn't your cup of tea) what I was doing until they released this button was using a diff code block:

Same change request but as diff md code block


Images generated by leonardo.ai

Thanks for reading 💚.

💖 💪 🙅 🚩
manuartero
Manuel Artero Anguita 🟨

Posted on July 24, 2023

Join Our Newsletter. No Spam, Only the good stuff.

Sign up to receive the latest update from our blog.

Related

Two tips for improving your Code Reviews
productivity Two tips for improving your Code Reviews

July 24, 2023