The 10 Code Review Commandments

Master the art of code reviews with these 10 essential rules to make you a valuable reviewer!

_______________ T A G S _______________

#programming #coding #webdevelopment #technews #programmerhumor #agile #codereview #opensource #howtodocodereviews #howtoreviewcode #codereviewbestpractices #codereviewtips #softwaredeveloper #howtoreviewpullrequests #howtocreatepullrequests #whatarepullrequests #codingbestpractices #softwareengineer #programmer #coder #nitpicking #codeperformance #improvecodeperformance #codereadability #codingtips #programmingmemes #programmingtips #programmerhumor

[farzany]

Hey, thank you for watching! I hope this was an entertaining and helpful video 🎉 There are so many great comments being made; I'm learning from you all! I want to mention that this list isn't in any specific order - it's more of a collection. It's important to prioritize these rules based on the needs of your project and team. As @milksaboteur said below, "you have to pick your battles"!

[souleater]

1. Readability
2. Performance/Scalability
3. Scope
4. Edge cases
5. Test coverage
6. Refactor repetitive code
7. Don't nit pick
8. Constructive feedback
9. Encourage discussion
10. Don't block for minor issues

[trimalakismeno]

"you're not a linter" 😂

[piroman]

Nitpicking is important with new members to make sure they learn to code the same as or similary to the rest of the team.
If that doesn't work I guess there's always fixing it yourself while thinking lowly of them.

[TawaraboshiGenba]

I strongly disagree with #2.

More often than not, you're unable to assess performance form just a code review.

You can only find out what parts of your code are prone to optimization by measuring performance.

#2 should be test coverage because optimized code is often more complex and unreadable, so optimizing should be done once your feature is implemented and you have proper test coverage to check for regressions.

[xpeter]

Very good list I have a different number 1: "Honesty - assess that every piece of code does what you would expect based on its name".

A non working functionality is a bug today, and non matching name is a bug tomorrow, when the next developer accidentally misuses the code. It is amazing to work in a code base where everything does what you would have guessed based on its name.

Amazing job on this video. You not only dropped huge truth bombs but also made it incredibly entertaining to watch!

[Nixxen]

Number 10 should be blocked by project style guide tests, and should not even reach a reviewer before it has been fixed.
I'd rather someone block my pr for having badly named public facing members, than having to clean up N files later because this typo or incorrect style is being used in most of the codebase.

[heatheranderson]

Top quality tips - your coworkers are lucky to have you (and to be providing content for these awesome videos 😂). Also, hello production!

[dorktime]

Agree with everything except "You will likely never see this code again" sounds like something a contractor would say before passing an unmaintable codebase over to a business that will be plagued with technical debt for years or decades.

[etooamill]

that first function rewrite example was unnecessary, the whole thing is only 7 lines of codes, all related one to the other. Splitting them up would make it more difficult to follow what the thing is doing as you have to jump to every definition while remembering where you came from and what the function is supposed to do

[stuartallen]

This is such a fantastic video, both educational and humor wise. I really wish code review etiquette was taught in college since i definitely embarrassed myself on my first couple.
Based on the view count i think you've found a content and editing style thats gonna blow up soon! Well done, I'm proud to be an early subscriber!

[thomac]

Having coding standards and automated tools checking and fixing stuff, cuts down nitpicking and personal preferences from PRs a lot.

In any case #8 is the most important. You don't want people be upset because they feel attacked in code reviews, it's a fast way for infighting or people leaving. Team leaders should be vigilant, especially when there are people with different cultural backgrounds and sensitivities.

I will add, not everything can or should be framed as a suggestion. What is important is to keep the tone polite and technical. When you can't say "in my opinion", do specify the rule, standard, pattern, problem, etc. the change you are requesting would address. And don't be afraid to sit down with your colleague to review his code together, a more organic discussion is less likely to generate misunderstandings compared to a comment without too much thought.

Basically, keep the tone positive as "let's improve this" rather than painting other people's work negatively.

[hmngghh]

Love this video, I was honestly surprised that it only has 2k views. While I don't work on github projects too often (I really should) I have dealt with stuff like this on a dev team I'm in, more specifically with a specific senior dev that was really nitpicky and tried to stop me from doing alot of stuff for.. his own ego to be honest (while his own code was, to say the least, not the best). He ended up getting fired for unrelated reasons, and it genuinely felt super freeing. I know this wasn't discussed in the video but, high egos really will put a stump in advancing a project due to stuff like this.

[LCTesla]

new rule: if you're going to suggest a non-functionally altering refactor, just do it yourself and make a counter-MR.

[wtf-smiley]

That trina girl sure is bringing some really bad memories to me. Nice video btw, you'll be growing on yt fast with this quality content.

[americanonobrasil]

Great video, bro. I'm about speed and getting out more iterations but my partner is a bit nitpicky. Gonna show him this and see if we an get somewhere in the middle 😂

[alexk]

Was impressed to see that this vid only has 1k views, such a good video! Keep it up 👌

[natenatters]

Love this! Do you have a copy of that narration document? I want to copy and paste it at my company too :)

[EvilCoffeeInc]

I do make remarks about formatting in my team's PRs, not because of my preferences but because we have team-wide formatter and linter settings for the sake of consistency. There is one dev on my team whose commits consistently include formatting-only changes or weird formatting errors and I make note of them because it means he has his tools configured incorrectly.