How to do a code review? Part 2: review navigation, speed, comments, conflicts

The first part of the article highlighted aspects of code review standards and points that need to be addressed first. In the final part, let's talk about:


  • review procedure
  • speed (and what it affects)
  • how to write comments,
  • discussion during the review.

1. Navigation through the list of changes in the review


TL; DR


Now that you understand what you need to pay attention to , you need to decide what the order of the review is. There are 3 main stages:


  1. Understand whether the presented functionality is needed in principle and whether it has a good description ;
  2. To pay attention to the most important thing in the modified code and how well the solution is designed as a whole;
  3. View the remaining changes in the order you see fit.

1.1. Change Overview


Look at the CL description and its main functionality: are these changes needed? If the proposed functionality is not needed, try to report it as soon as possible and explain the reasons. In this case, it is advisable to offer the developer a task in return.


: " , , , FooWidget, . . , BarWidget . ?"


, : CL . : , .


, , , โ€” , . "" , , .


1.2.


CL โ€” , . โ€” . CL , , CL .


CL, , CL . , , โ€” .


:


  • CL . , .
  • . , , , .

1.3.


CL , . . , . โ€” .


2.


2.1. - ?


Google , , , . , .


, - :


  • . , , , . , , , , .
  • . , . . , , , CL , . , .
  • . , . , CL.

2.2. ?


- , , .


โ€” ( , , ).


, CL ( ) .


2.3. vs


, : - , , , . , , . , , , .


: , , , .


2.4.


-, , , , , CL . , .


, , .


, , , CL , . . , โ€” .


, , . , , .


2.5.


, , , , . , .


2.6. LGTM (looks good to me = )


, , CL . :


  • , .
  • , .

LGTM , "LGTM, Approval".


2.6. CL


CL , CL CL, . , .


CL , , CL, , . , , .


2.7. -


, , . , CL . . , , โ€” , .


2.8.


, CL . . , , " ".


3.


TL;DR


  • .
  • .
  • , , .
  • , .

3.1.


, , , , . โ€” , . , , , -, . :


: " , ?"


: " , , . , , ."


3.2.


"" , . , โ€” , .


3.3.


CL, . .


. , , . , , , , , , , , .


, . , CL , โ€” , .


3.4.


, , , . , , , , .


, , . : , , , , , .


4.


, .


4.1. ?


, โ€” , . , . ? ? , .


, . , . , - .


, , , . .


, . , . , .


4.2. ?


, , . , , , , , . , , , . , , .


4.3. " "


( ) , . , - CL, . CL . , , CL, - . , . , โ€” , , , "". .


CL , CL , . CL , , , , TODO- .


4.4.


, , . .


, , , .


Even the most ardent opponents of strict reviews can become your biggest allies when in practice they encounter a situation proving that your tough position in the review was correct.


4.5. Conflict resolution


If you follow all the above rules, but there are still conflicts with the developers and you cannot solve them, refer to the Code Review Standards to understand the practices and standards that can help you resolve the conflict.


All Articles