DEV Community

Irinabert
Irinabert

Posted on

Code Review Antipatterns

Sarcastic advises for the vital process of software development. Because sometimes irony is the best teacher.

Be personal

Do you remember John didn't buy you a drink last time you met for a social? So reviewing his PR is the right time to show him how wrong he was. Challenge every idea, every possible variable name, and every white space in the PR. He deserves this.

Be excessive

That's right. Don't keep it. All thoughts, ideas, questions - put it all there. Just ask yourself, when was the last time you asked Anna how are her kids? Well, PR is the perfect place for it. What else can we add? Weather comments, photo of your pet, last update on your favourite football team. Let it be there.

formatting is our enemy
we can save a lot of time ignoring all of the commas bullet point highlighting structure bolding markdown they are there to waist our time serve no purpose at all

Be unreasonable

"I just don't like it" is a perfectly valid reason for a PR rejection, because your word does matter.

Michael office I don't like it

Be toxic

Try to put your colleague in their place with sarcastic comments in PR, answer the question with the question or use as many rare technical definitions and abbreviations as possible. They will scare, doubt themself, and lose confidence. That's the best way of learning.

Toxic comments

Compare everyone with everyone

Don't forget to put a comment that Rachel did a similar task quicker, better and had no comments at her PR. You always have someone to compare to: Bill Gates, Mark Zuckerberg, Elon Mask, Linus Torvalds. You just need to find the right person for the situation.

Discuss PR privately

We are all friends here, right? So use personal chat to ask Jimmy to fix this line 78 in Program.py. No comments in PR, no bureaucracy, no problem. Plus, from the outside, Jimmy's PR will look perfect. Like there were no issues at all. It is a win-win situation ;)

Never resolve comments

If there are people that use comments in PR, teach them a lesson and never resolve them, so they can get lost in their comments and never finish PR.

Hints instead of explicit examples

Use PR as game time and leave some breadcrumbs and hints instead of explicit examples of what you meant so the PR owner can guess the right idea. You are there to help.

P.S. Please add your antipatterns in the comments :)

Top comments (6)

Collapse
 
bcouetil profile image
Benoit COUETIL ๐Ÿ’ซ

Interesting article point of view ๐Ÿ˜…

Other anti-patterns I can think off :

  • always approve without question/comment
  • self approve before anyone has the time to review
Collapse
 
irinabert profile image
Irinabert

Ha-ha-ha. Good ones ๐Ÿ˜„

Collapse
 
laurencenairne profile image
Laurence Nairne • Edited
  • Always be sure to request changes that have nothing to do with the PR
  • A PR is the best place for a new dev to discover the team has coding standards
Collapse
 
tingwei628 profile image
Tingwei

Hopefully, I could go through these obstacles ๐Ÿ™ƒ

Collapse
 
typehorror profile image
Type Horror

Oh man, this is goldโ€ฆ I definitely feel glad I didnโ€™t get to face these too often even if it brings back a few bad memories

Collapse
 
rain_32 profile image
Neer

Or use mindreader.js to read the mind of your nearest reviewer