DEV Community ๐Ÿ‘ฉโ€๐Ÿ’ป๐Ÿ‘จโ€๐Ÿ’ป

Cover image for โœจToday I Learned: The Subtle Art of Code Reviews ๐Ÿ’กโœจ
Samina Rahman Purba
Samina Rahman Purba

Posted on

โœจToday I Learned: The Subtle Art of Code Reviews ๐Ÿ’กโœจ

Code reviewing is hard. The pull requests I reviewed were for the My Photohub project.

Code reviewing for the first time in my life made me realize how difficult the task is. As a first-time code reviewer, I found myself staring at the code, pondering about what could be done to make it better. Feeling rather lost. I came up with some mediocre suggestions. Nothing too great. Nothing that I felt truly satisfied with suggesting. Then came professor Dave to do his code review on the same pull request I was reviewing. I just sat there astounded at the details he looked at and all that I missed. He suggested major things such as better programming logic, and minor things such as updating the project description. I could have suggested those minor issues too; a better project description, a better logo that goes with the project instead of the default React logo, and creating follow-up issues for certain things. Why did I overlook the obvious, which professor Dave so elegantly and effortlessly pointed out?
Well, I was so fixated and overloaded with trying to find something big to suggest that my senses had completely shut down on the easy stuff. This made me realize just how much I need to practice reading other people's codes and thinking critically about them. Like attaining mastery in any skill, being a good code reviewer is a skill that takes time, patience, and practice.

Some code review best practices ๐ŸŒฑ

This is a great article I came across while researching for the code review best practices.
It beautifully elaborates on how design, functionality, complexity, testing, naming, commenting, style, consistency, documentation, and context are all important components of a code review.

Time. Attention. Communication โณโ€ผ๏ธ

A good review takes time and attention. It requires us to think critically about the programming logic. For beginners, it can feel overwhelming to stare at the code someone else wrote to suggest something meaningful. There is a fear of being wrong. A fear of offending the person writing the code. I know, because I felt all those. However, in open-source development, it is always important to remain polite and respectful in communication while giving constructive feedback. We can sometimes disagree with the review but if we do so, we should communicate that politely.

It is also important to keep an open mind while giving and receiving feedback. It is through this exchange of ideas, and feedback that we can improve over time.

If you have some coding experience and spend enough time and give undivided attention to the code, it is almost always possible to come up with suggestions for improvement.

Start small ๐ŸŒ ๐ŸŒฑ

If you are a beginner, start small and learn from others. I learned a lot of important things by reading professor Dave's reviews. Then I realized that it is by seeing how more experienced people review code, that I can get better. I made some notes of the things I learned by reading a lot of his code reviews on GitHub. I turned those things into some sort of a checklist:

  • Ask specific questions regarding certain lines of code, if unclear.
  • Look for unused dependencies and ways to optimize package.json.
  • Look at the code from the context of the entire codebase.
  • Leave some words of encouragement to point out what the developer is doing right!
  • Focus on functionality and compatibility.
  • Look for clear names and proper naming conventions.
  • When reviewing README or CONTRIBUTING.md files, look for consistency in style, formatting, spelling, and grammar.
  • Make sure that the design and UI look good.
  • Ensure that the unit tests are appropriately designed for the code.
  • Make sure that the code conforms to a certain style guide provided.

My code reviews

Here are the code reviews I did over the last few days. Click on the links to view the entire review :)

While suggesting changes, I also provided the rationale behind suggesting the changes and asked for the developer's opinion. This kept communication professional and polite.

Review 1

Image description

Review 2

Image description

In linters we trust ๐Ÿš€

Automated code checkers and static tool analyzers can help a reviewer save a lot of time looking for silly typos or stylistic errors. Linters and scanners act like spell and grammar checks for a code. With the headache of formatting and other stylistic issues out of the way, the reviewer can focus on the actual logic of the code. As code reviewers, it is essential to make use of the automated code checkers available to save time and sanity.

Closing thoughts on productivity ๐Ÿ’ญ

If you are someone like me who loves reading about psychology and productivity, this is a great article!

Top comments (26)

Collapse
 
efrenmarin profile image
Efren Marin

I think the toughest part for me is phrasing my findings as polite as possible without it coming off as rude.

Collapse
 
nagkumar profile image
Raja Nagendra Kumar

Just put all code review insights in the jira tool.. and give the owner to decide..that way egos can be avoided.

Collapse
 
saminarp profile image
Samina Rahman Purba

Yes that is difficult sometimes :)

Collapse
 
mmi profile image
Georg Nikodym

tl;dr Code review, viewed as a kind of "lunch and learn" thing, has value.

I observe that people get too hung up on the code review as a thing to drive code quality. That's important for sure. But the other benefit of reviews is a way for a team to learn things -- whether that's coding techniques, patterns, new tricks, how their project works, whatever. So, rather then viewing code review as hard or a chore, consider it an opportunity to level up your skills

Collapse
 
saminarp profile image
Samina Rahman Purba

Very well said :)

Collapse
 
souksyp profile image
Souk Syp.

I always pull my coworkerโ€™s branch locally to see better. ๐Ÿคฃ

Collapse
 
saminarp profile image
Samina Rahman Purba

Good idea

Collapse
 
zoten profile image
zoten

Good suggestions, thanks!
I'd like to give a lot of credit to the way of approaching communication: positive feedback, attention to the developer's opinion and always think about what they may have thought while writing that line are the basis of a positive feedback cycle :)

Collapse
 
saminarp profile image
Samina Rahman Purba

Thank you :)

Collapse
 
octav profile image
Octavian Voicu • Edited on

Well, I was so fixated and overloaded with trying to find something big to suggest that my senses had completely shut down on the easy stuff.

It may help to not think that you have to find issues; focus on understanding the code first. If you're having trouble understanding a piece of code, others reading it may encounter the same issue and you should probably ask the author to improve it even if you eventually figure it out. It may be worth trying a bit harder to understand the intent to be able to give better feedback (e.g. suggest a change that would have helped you figure it out 2x faster), but it's also valid to just state that you don't understand something (especially if you're short on time). The converse is also applicable: if a reviewer asks a question and it's not a trivial misunderstanding, try to answer the question by improving the code so the answer becomes trivial (or simpler). Do this even if they didn't request any changes. Think about how that code would be more readable: adding a comment, improving naming of function/param/var, defining a constant to make magic values self-documenting, restructuring code with additional helper function, improving testability.

Don't forget to look at the big picture (really, start with that). Good change descriptions start with self-contained one-line summary of what's being done and then additional paragraphs as needed to elaborate on the 'what' and also the 'why'. Don't need to write much about the 'how'--that's the change itself. If it's impossible to write a one-line summary, the change may be too big. Avoid useless/filler phrases (e.g. "fix bug", "refactor code").

Don't start nitpicking on formatting or minor details if the change is adding something that shouldn't exist in the first place (e.g. some redundant functionality), or if there are high level changes that need to be made which will probably result in a major rewrite of that piece of code.

Collapse
 
saminarp profile image
Samina Rahman Purba

thank you for your wonderful comment and insights :)

Collapse
 
iurijacob profile image
Iuri Jacob

Loved the politeness of your comments. I must keep this in mind next time I'll be reviewing. ๐Ÿ™‚

Collapse
 
elsyng profile image
Ellis • Edited on

My personal approach to code reviews:

  1. PR's should be prio 1, they should be done first, so people don't wait.

  2. The reviewer should:
    -- add their suggestions,
    -- approve the PR immediately and automatically,
    -- trust and leave it to the other person to process the suggestions as they see fit,
    -- and don't chase it (don't double-check what they've done afterwards).

With the exception of important and critical issues, if any, of course.
:)

Collapse
 
tdaw profile image
TD

Great post ๐Ÿ‘

Collapse
 
saminarp profile image
Samina Rahman Purba

Thank you so much for taking your time to read

Collapse
 
le_woudar profile image
Kevin Tewouda

nice article!

Collapse
 
hiholas profile image
Nico Delgado

thanks!

Collapse
 
scottbeeker profile image
Scott Beeker

Great Stuff!!

If anyone want's to follow each other on github here's my handle
github.com/dangolbeeker

Collapse
 
ranggakd profile image
Retiago Drago ๐Ÿ‡ฎ๐Ÿ‡ฉ๐Ÿ‡ต๐Ÿ‡ธ

I wonder whether it could be applied on data-related or AI / ML code

Collapse
 
verdelmanzano06 profile image
Verdel Manzano

Great!

Collapse
 
xuanlocle profile image
xuanlocle

Great idea

Collapse
 
batunpc profile image
Batuhan Ipci

Lovely ๐Ÿคฉ

Collapse
 
saminarp profile image
Samina Rahman Purba

Thank youuu

Fun with console.log()

>> Check out this classic DEV post <<