How I learned to make the world better in HeadHunter

Before I came to HeadHunter, I did not know what code review is. I knew what code approval was - it was in one American company, where I started my career, and where all the code in the project passed before the wise eyes of Professor Fortran at a table in the back of the office. He looked at my first steps in the development with a paternal smile and said: “Here you can correct, please, and you can release it”.




I also knew that such a “blessing of the tmlid” was like this at my next job, already in a Russian startup, where the quality of the code was controlled by a gray cardinal who secretly visited my branches in the repository and contributed his corrective commits there. Sometimes he didn’t delete my lines, but commented on them, leaving notes like “Who wrote it - never do it again” or “Made it easier”, which clearly indicated to me my place in the corporate hierarchy and in addition caused me a burning shame, a teenager who has forgotten to erase the history of the browser on the family computer.
')
It all ended up like a bad dream when I got a job at HeadHunter.

“We have a review code,” the front-end leader told me at the interview.

- Wow, cool! - I replied, remembering that you can never show that you don’t know any term.

Code review is a practice that allows you to maintain the quality of the code in a project by reading it out by other developers before releasing the task. This means that each programmer, having finished coding, tells his colleagues: “Guys, I did this and that here - please, look, check that everything is in order, and I am not mistaken anywhere.” Colleagues run over the eyes of its code, mark typos, semantic, logical and technological inaccuracies, offer to correct them, offer more effective solutions; the developer agrees or disagrees with them, they find a compromise, someone alone is appointed as the responsible reviewer, after which the clean (as far as possible) code is sent to production.

The review is a fascinating and complex process, almost like a development. Depending on the qualities of the task (complexity, interestingness, potential for holivars and mutual trolling, the actual code) between two and several dozen developers can take part in the discussion, and it can last from a couple of minutes (“Hello, there is a task ...” - “Reviewed”) up to weeks (“Re: Re: Re: Re: Variables in the global domain view ... [23:40]”).

There are many ways to review code. Someone prefers the version of pair programming, when adjustments are made directly during the discussion. This can be useful when the task is nontrivial, and, before discussing the details, you need to understand the essence of what is happening. But it is usually more convenient to work asynchronously and lead a discussion in the comments to the code, while at the same time doing other things. Among the front-end hh.ru, the second option is much more popular. First, because it is beautiful.



Secondly, because it is always possible to turn to other developers and jointly resolve the dispute.



Thirdly, because in this way we kill two birds with one stone: we keep the code clean and write documentation that will help in the future to understand why certain lines were added.



Find an author.



Understand it.



The review certainly has its drawbacks. The main - an increase in development time. If you are working on a prototype or MVP, and you need to do "quickly and in order to work," then a review with its temptations to find fault with trifles can only spoil everything. That semantic quibbles are the most common and at the same time the most annoying. As a vampire who first tried human blood, I instantly tasted after the first extra space I came across. A week later, I was already ruthless Grammar-Nazi, did not miss a single random transfer - only PEP 8, only 79 characters per line.



Another significant drawback is subjectivity. There are algorithms, models, technologies, and there are habits, oddities, and simple tastes: a colleague can assume that your approach is wrong, inefficient, ugly, and no arguments will be applied to it. The rule that the last word in semantic disputes remains with the author helps very well here.

In general, with all the side effects, review is a very useful tool, especially in large companies with a long-term code base and a large staff of developers.

If you want to implement this practice in your project, then a good start will be familiarity with specially designed tools for this (for example, Stash or Upsource ) - so as not to reinvent the wheel and ultimately do not slide back to the “I corrected with you” approach. Also, if you are using GitHub, you can try to create a pull request for each new branch: it can be a simple description of the task in a nutshell or mini-documentation in the “problem - reason - solution” format with screenshots of the “was - has become” type (especially if we are talking about layout). At first, this can be done with an option, and then included in the standard workflow, as we have done: if there is a task, there must be a corresponding pull request.

And when there are too many perfectionists and hunters in the team, utilities and plug-ins for IDE will come to your aid, keeping track of formatting and syntax even at the design stage. The main thing, as in everything that relates to the development, is not to get carried away and not move out into overengineering - and then, undoubtedly, you will have a chance to make the world a little bit better.

@dude No, let's write better "a little closer to the ideal."
@vanowski No, it sucks, so do not say. Simply - “better” and all.
@dude Okay, please quote and dash, please, and dots over e -

(Captions.)