Recipe useful code review from a developer from Yandex

Hey. My name is Sergey, the last five years I have been working at Yandex. During this time he participated in the development of eleven projects. Wrote code in javascript, python and c ++. Some projects were done alone, others were developed in a group of eight people. But in every team, on all projects, regardless of programming language, I used code review.

With the help of code-review, I constantly learn something new. Sometimes, looking at someone else's code, I want to exclaim: "And what, is it also possible?" In someone else's code, I find interesting tricks and take them into service. Many new knowledge scoop from comments to my code. It was a discovery for me that people love to share their experiences. Even when I develop a project alone, I ask the guys from the other team to see my pullrequest. It motivates to write beautiful and clear code.

But it was not always so. Once a review was a punishment for me. I could write the code for a week with inspiration, putting all my energy into it. I sent a pullrequest, pinged the reviewers three times, and in return received a dry “like ok” or, even worse, dozens of comments on the merits.

I came to the review pools of five thousand lines. I spent hours trying to figure out the code, scrolled from function to test and back again a hundred times. He wrote dozens of useless comments about the missing semicolon. All this terribly annoyed me. I often put off reviews for later, and I had dozens of unwatched pools accumulating.

If you felt it on yourself, then the article is for you. Today I will talk about the techniques and tools that I use every day for five years, a daily code review.

"Before the review." Tips for the author

Imagine that the solution to the problem is cooking. You work in a team, so you need not just to cook, but also to teach this to other chefs. It is not enough to show them the result, you need to write a recipe.

Commits

Each step of the recipe is a commit: two eggs were broken — commited, a glass of milk was added — commited, two hundred grams of flour were poured — again commited.

In each commit I express one simple thought. This can be an implementation of a model method or a component in layout. So it will be easier for the reviewer to understand me. I do not blame it on the whole task, which can not be swallowed at a time, but I talk about the decision in pieces.

Any refactoring I put in a separate commit. Often, refactoring is purely technical in nature, for example, renaming a method. The viewer does not need to read every line of such a change. He runs his eyes "diagonally" and will be able to devote more time to the more important code.

Crumble, break, chop your code into small commits. This will allow the reviewer to better understand your code. It's okay if you overdo it with decomposition. Two commits are easy to merge into one. It is much more difficult to divide a large commit into several small ones. "Chopped vegetables" is easy to get by mixing "chopped tomatoes" and "chopped onions." But to get all the ingredients from a plate of salad, you need to spend much more time.

After the commit, I immediately push the changes to the githab. It rescued me a couple of times when there were "coffee troubles" with a laptop.

Description of commits

When I write an email, I fill in the title and contents of the letter. The heading is a short and capacious title, the body of the letter is a detailed description with pictures and links. I apply the same approach to the description of kommit.

git commit -m 'fix1'. git commit, . ( ). ( ).

, " " " ". , . . ASCII- . , , .

( ASCII-)

( . vim)

( GitHub. )

, , . :

git status
git diff comments.js
git add comments.js

, . git add ., . , .

. , . JavaScript ESlint. R2-D2 " ", . , , .

WebStorm " ". , , . husky.

. . .

( )

– , – . , . git log --pretty='%h: %B' --first-parent --no-merges --reverse.

( `git log --pretty='%h: %B' --first-parent --no-merges --reverse`)

( , )

, . , . .

! , .

« ».

– , .

, . , , . , . , " ".

. . .

–

. ! . , . , , . , .

. , . .

. :

function sum(arr) {
  return arr.reduce(function (res, i) {
    return res + i;
  }, 0);
}

sum([1, 2, 3]); // 6

, :

const sum = arr => arr.reduce((res, i) => res + i);

, . , , . , . – .

Offline

, . , . .

offline-. (, : , ), .

. -, . -, : ( ). -, , — .

, . offline- .

« ».

, . . .

, . , , , . , : ? ? ? , .

. , .

( : )

. , . ? ' , .

-. . , . , , .

git rebase --interactive master. , FEATURE-1, master — . , , pick squash. .

( , )

--force. , . git push origin FEATURE-1 --force , . , .

GitHub. "Squash and merge".

( GitHub)

, . FEATURE-1. :

git checkout master
git pull origin master
git branch -D FEATURE-1

( .)

, , :

#     
git checkout -b FEATURE-1

#  
git status
git diff src/controllers/v1/comments.js
git add src/controllers/v1/comments.js

#    
git commit
git push origin FEATURE-1

#    
git log --pretty='%h: %B' --first-parent --no-merges --reverse

#    
git rebase --interactive master
git push origin FEATURE-1 --force

#  
git checkout master
git pull origin master
git branch -D FEATURE-1

#  ,      
git add comment.js

#    
git commit --amend