How To REALLY Do Code Reviews

► Timestamps
00:00 Intro
01:07 What Is a Code Review
02:48 Levels
05:26 Ego
06:13 Philosophy
07:19 Project Type
08:24 Location
08:59 Time
10:38 What We Will Review
11:51 Review Style
13:02 Inspecting a Pull Request
19:04 Giving Feedback
22:44 Outro

► FOLLOW ME

[__Name_It__]

you haven't mentioned the case of two juniors reviewing each others' code 🤦‍♀ that's when the ego goes above and beyond.
thank you for this video!

[djxak]

Thanks for the video! And please don't consider comments to the previous video as "mean" or "rude". Please follow your own advice about ego. Those comments basically are "Code review" of your **video** and not attacks to you as a person.

I can't say the previous video was bad or not useful. It was great. Just **named** incorrectly. I (and considerigng the comments, many others too) expected to see how to properly do a code review (based on the video's title), but instead saw how to do a refactoring. :) That was also very entertaining, actually. I watched to the end. This says a lot. So, please don't be frustrated by these comments. It was "righteous anger" from developers (including me), who still beleive in unicorns and in existence of easy one-size-fit-all way of doing a code review. :) The title deceived these expectations. Like these titles of news in yellow press, when title is specifically crafter to be clicked, while has nothing to do with the contents of the article.

And I honestly understand how this happened. It was really much easier to do a refactoring yourself (the 3rd way of "code reivew" from this video :-) ) than do a long and very time-consuming and also stressful and frustrating (because it's hard to explain in words something, that should be explained in a code) review. And also because, as you said, if you would not do this refactoring yourself from start to finish, you will not come to solutions you came in the previous video. It's hard to see all possibilities to improve based on the 1st version of the code. Usually it's a iterative process (so, the ping-pong). But who has **that much** time for code review of such small feature? :) And yes, this is very small and simple feature. They are not always that small and simple. :( And divide to smaller chunks is not always possible too.

The content of **this** video is much closer to the title! :) The thought about dependency of the style of code review on the type of a project was very interesting. Yes, it's obvious, but I never thought about it, just did intuitively.

Anyway, keep up the good work!

[artellandoe-solucions]

I feel somewhat a bit of a picky prick doing reviews. First I read the comments, then take a light look to the diff, add the comments that come to my mind reading... And then I pull the branch, run some tests, maybe go back to the issue description... If I see too many things, I prefer to have a conversation/call with the colleague instead of putting a long list of comments that could be felt like a long list of denials. I think it feels better having a conversation. In fact, lots of times I am the one doing/testing something wrong, and that helps me to get the point. If there are some specific things to point out, I use the comment. Then, another look at the diff. And if other reviewers add comments, I review them, and maybe they could raise a new comment now that I am aware of something, new ...
But for me, the most important/complex thing to deal with is when there's a draw: you have your point of view, and the other side has other. And the amount of reasons I had found is quite huge:
- sometimes you cannot argue too much because the other side does not have the same depth of view than you have. And I´ve been there... I mean, when a domain expert is reviewing your code and you joined the team 3 months before, you feel like you are completely blind. But the reviewer is sooo busy that it is necessary to find a balance to make your addition profitable, and a step forward. In that case if I (as the reviewer) don´t see something that could hurt the project in the short term, and assigned tasks promise me to be able to touch it and refactor in the future, I choose to not struggle too much there and make the move a bit later. Because once that colleague can see the refactor (I will add her/him as a reviewer), she can get a new point of view, compare, and understand better the advantages and disadvantages, because he had worked on it, and is able to understand it.
- sometimes you have different backgrounds, readings, schools, interpretations ... But both positions are fixed. In that case, I choose to let the evolution of the code give an answer. And maybe it is neither A nor B, but C. But surely it will say something difficult to ignore :D
- If there's some kind of hierarchy, maybe I could think of asking to follow a strict guideline. But that's not something I like too much (except if there's something harmful). And for me it is more usual to have an horizontal team, with some exceptions (that is, those 2, 3, 4 ... people you need to be there when things start getting complex and you stop having answers for the questions). In that setup, for me it seems it works better using work as an example. You can suggest, and point to similar things you did earlier. And you can learn from the things your colleagues have done when you review their code. And it is more enriching than some long discussions without a clear winning argument.
I hope it could help someone, or raise some interesting comments to learn from :)

[Mac-vnrf]

Thanks for video. have you used any AI tools for code reviews?

[theceilidhboy]

How about a 4th option: add a comment with your suggested changes using ```suggestion...``` and ask the pull requester to consider committing these changes to their PR branch?

[windowsrefund]

Way too much value being put on code reviews here. Much can be delegated to functional testing. I swear, if someone ever wanted to "meet with me" in order to do a code review, my first thought would be "this guy needs to get a life!". No disrespect intended. So far (10m in), I'm enjoying this conversation as I appreciate the issues you're bringing attention to. By the way, I'd ask you to please lose the "open source" and "commercial" adjectives when describing software. There are only 2 types of software; those being Free (freedom-granting) and Non-Free (freedom restricting).

[deconcoder]

This is not about code review??!! This is about social pathology. Who is on top of who, how to manipulate people without having the object go ape on you.

I bet this guy's reviews are full of patronizing nonsense. Sorry gonna go unwind by rereading Bentham's piece on the Panopticon...