Revisão de código: você está fazendo errado


Hoje, muitas pessoas usam revisões de código em desenvolvimento. A prática é útil, necessária. Mesmo se você não estiver fazendo uma revisão, provavelmente já sabe o que é.

Existem várias ferramentas para revisão de código no mercado, com cenários, recomendações e regras de uso prontos. GitHub, Phabricator, FishEye / Crisol, GitLab, Bitbucket, Upsource - a lista continua. No Badoo, também trabalhamos com eles ao mesmo tempo: no meu artigo anterior , contei nossa história sobre revisões de código e como criamos a invenção de nossas próprias “bicicletas” - soluções Codeisok .

Há muitas informações, você pode pesquisar no Google vários artigos sobre revisões de código com exemplos, práticas, abordagens reais que falam sobre quão bom, quão ruim, como fazer e como - você não precisa, o que precisa considerar e o que não, etc. e) Em geral, o tema é "sugado até os ossos".

É por isso que a outra parte do iceberg pode não ser percebida.

Espero que você leve a sério as coisas que descreverei neste artigo, em alguns casos deliberadamente exagerando. Uma revisão, como qualquer outro processo, requer implementação cuidadosa e deliberada, enquanto a abordagem "Faremos o que todos os outros, se for o caso", pode não apenas falhar, mas até prejudicar.

Depois de ler esta introdução, você pode ter uma pergunta: o que é complicado na revisão de código? O ponto é insignificante. Além disso, a maioria das ferramentas listadas acima imediatamente oferece uma revisão do fluxo (pronto para uso: defina, confirme / inicie - e pronto!).

Mas a prática mostra que nem tudo é tão óbvio quanto parece à primeira vista. Inclusive devido à simplicidade imaginária do processo. Quando tudo está funcionando, existe o risco de o gerente se acalmar e parar de afundar no processo, passando-o para os desenvolvedores. E eles seguirão o processo ou farão outra coisa em detrimento da resolução dos problemas que o processo de revisão foi projetado para resolver. O gerente pode não estar ciente de que algo está errado, porque não define as regras nem impõe regras. Embora seja muito importante para uma empresa resolver problemas o mais rápido possível, sem perder tempo em atividades não planejadas .

Era uma vez, quando eu trabalhava em outra empresa, fiquei profundamente impressionado com a conversa sobre revisões de código com um dos principais desenvolvedores. Foi p1lot . Talvez um dos leitores esteja familiarizado com ele (p1lot, olá :)). Atualmente, ele está trabalhando conosco no Badoo, o que é ótimo.

Então, o PILOT disse: "A coisa mais difícil para mim no processo de revisão é comprometer e pular ainda mais a tarefa concluída e funcionando corretamente, mesmo que eu conheça outra maneira de resolvê-la e que goste mais do meu método". Na minha opinião, este é um pensamento-chave. E a principal mensagem de todo o meu artigo gira de alguma forma em torno deste postulado.

Por que preciso de um processo de revisão de código?


Você tem um comentário em sua empresa? Você está fazendo certo? Eu tenho um teste que pode fazer você duvidar.

Você precisa responder apenas três perguntas:

  1. Quanto tempo leva uma revisão de código para uma tarefa média (esférica no vácuo)?
  2. Como você minimiza o tempo de revisão?
  3. Como você determina se uma revisão de uma tarefa específica é feita corretamente?

Se você não tiver respostas claras para algumas (ou todas) perguntas, se duvidar de suas respostas, ouso assumir que algo está errado.

Se você não sabe quanto tempo levará para revisar e não a minimiza constantemente, como planeja? É possível que o artista, que fez a revisão por quatro horas, não tenha bebido chá pela metade desse tempo? É possível ter certeza de que o tempo para beber chá não aumenta de tarefa para tarefa? Ou talvez o revisor não veja o código, mas simplesmente clique em "O código está bom"?

E, mesmo que a revisão seja feita de boa fé, como podemos garantir que, com o crescimento da base de códigos e da empresa, o tempo para concluir as tarefas não aumente? Afinal, o gerenciamento, em regra, espera que os processos se acelerem, e não diminuam.

Se a resposta para a terceira pergunta não vier à mente imediatamente, faz sentido perguntar mais uma. Você sabe por que você realmente precisa desse processo? Porque "é tão habitual para todos os grandes"? Talvez você não precise dele?

Se você perguntar aos seus líderes e desenvolvedores sobre o que é a revisão de código "correta", ficará muito surpreso com o quão diferente você ouve. As respostas podem variar de acordo com a experiência pessoal, crenças, confiança pessoal na correção ou incorreta das coisas, e até mesmo na pilha de tecnologia usada e na linguagem de desenvolvimento.

Lembra das ferramentas prontas para revisão de código, oferecendo suas próprias abordagens e fluxo? Por exemplo, eu me pergunto como os desenvolvedores dessas ferramentas responderiam à pergunta. Suas respostas sobre a “correção” da revisão estão correlacionadas com as respostas de seus funcionários? Não tenho certeza. Às vezes, infelizmente, vejo como alguém implementa ferramentas de revisão em casa, sem duvidar nem por um minuto de que elas são necessárias. As pessoas fazem isso "para mostrar" ou relatam que "eles implementaram a revisão de código, está tudo bem". E no final eles esquecem disso.


Não quero ser cap, mas mesmo assim. Ao se comunicar com os funcionários, preste atenção a respostas como "Porque é tão estabelecido" ou "Esta é uma prática recomendada, todo mundo faz". Você mesmo está ciente de que não precisa implementar um processo sem pensar, sem entender por que ele é necessário. Qualquer processo deve ser justificado e implementado (se necessário), ajustado às necessidades do negócio e do projeto, bem como aos problemas que realmente existem e que você realmente deseja resolver. O culto à carga em uma empresa com uma boa cultura de engenharia não pertence.

Portanto, é importante que o gerente transmita às pessoas a revisão "correta" do código, necessária precisamente em seu projeto. E antes disso, é claro, os critérios de “correção” devem ser formulados por você, escolher as ferramentas certas e estabelecer as regras apropriadas. E já existe perto de controle.

Revisão de código incorreta


Então, qual é o processo certo? Vamos acertar. Abaixo, há muito do meu raciocínio pessoal e, para aqueles que mal podem esperar para descobrir o que estou escrevendo tudo isso, proponho ir imediatamente para a parte "Revisão do código bom".

Aos que ficaram, agradeço e, no entanto, proponho determinar a correção da revisão por conta própria, com base nas necessidades do seu projeto, considerando cuidadosamente tudo. E eu apenas tento ajudá-lo com isso.

Para destacar coisas importantes e necessárias para você em qualquer processo, pode ser útil começar cortando coisas obviamente desnecessárias que prejudicam a cultura de engenharia. Então vamos lá.

Partilha de conhecimento


Para a pergunta "Por que preciso de um processo de revisão de código?" Já ouvi a resposta " Compartilhar conhecimento " muitas vezes. De fato, uma coisa útil e necessária. E muitos concordam que é importante ter um processo em equipe para a troca de conhecimento e experiência. Mas você tem certeza de que a revisão do código é adequada para isso?


Eu perguntei repetidamente às pessoas o que elas querem dizer com "compartilhamento de conhecimento". E em resposta ouvi coisas diferentes.

Em primeiro lugar, é uma demonstração para os recém-chegados (na equipe ou no componente afetado) das regras e práticas aceitas: é assim que é habitual fazermos isso, e não o fazemos, é por isso e é por isso.

Em segundo lugar, essa é uma nova visão de um iniciante (novamente em uma equipe ou em um componente afetado) sobre como as coisas comuns são feitas. De repente, eles estão sendo feitos de maneira ineficiente, e uma nova pessoa ajudará a descobrir isso?

Em terceiro lugar, isso é apenas uma introdução a alguma parte do código. De fato, se no futuro o revisor enfrentar a necessidade de alterar essa parte do código, será muito mais fácil para ele.

Vamos examinar todos os pontos e tentar avaliar a relevância deles no processo de revisão.

Revisão de código como uma ferramenta de treinamento para iniciantes


Nesse caso, estamos falando principalmente desse cenário: um iniciante recebe uma tarefa, ele a executa e, em seguida, um membro experiente da equipe (proprietário do componente) indica os erros que ele cometeu. O processo é importante e necessário, não discuto - os iniciantes precisam mostrar de alguma forma as regras aceitas na equipe.

Mas vamos ser honestos: não é tarde demais? Não seria mais correto contar ao iniciante sobre essas regras antes de admiti-lo no código? Afinal, o preço de um erro é muito alto - raramente os iniciantes trabalham imediatamente da maneira que você precisa. Portanto, a tarefa voltará e voltará para revisão. Portanto, o produto ficará disponível para o usuário mais tarde do que poderia.

Acontece que em um processo cuja duração é difícil de avaliar, adicionamos outro, cujos custos de tempo são ainda menos previsíveis. Assim, aumentamos o tempo necessário para entregar a tarefa ao usuário em uma quantidade ainda mais desconhecida.

Obviamente, às vezes o treinamento de iniciantes através de processos de trabalho tem o direito de existir. Mas, às vezes, não pensamos nas deficiências das abordagens que se tornaram um hábito. E cometer um erro aqui não é apenas fácil, mas muito fácil.

Por exemplo, se a empresa não possui um processo simplificado de treinamento e coaching , o gerente é forçado a "jogar na água" um recém-chegado. Ao mesmo tempo, o último não tem escolha: você deve "nadar" ou deixar a empresa. Alguém realmente aprende com essas situações e alguém não se levanta. Eu acho que muitos encontraram problemas semelhantes em suas trajetórias profissionais. Minha mensagem é que o tempo mais valioso pode ser gasto de maneira não ideal devido a esse fenômeno. Assim como o processo de adaptação de um novo funcionário em uma equipe, pode não ser o ideal. Só porque ninguém tinha as mãos para organizar um processo eficaz de introdução de novos funcionários na equipe, e o gerente pensou que o recém-chegado aprenderia tudo na fase de revisão do código. Mas, de fato, esses são dois processos diferentes, muito necessários e importantes.

É claro que, mesmo nas condições em que as regras foram explicadas inicialmente ao iniciante e que a empresa possui outros processos de treinamento, o processo de adaptação do iniciante precisa ser controlado de alguma forma. Uma revisão é um processo que pode ajudar. Mas controle e treinamento são duas coisas diferentes, certo? Além disso, todos os membros da equipe precisam de controle, e não apenas iniciantes. Afinal, todos nós podemos cometer erros, esquecer os acordos e afetar de alguma forma a parte do sistema que a equipe inteira possui (nesse caso, o código).

Que conclusão pode ser feita? O processo descrito acima visa atingir um objetivo diferente: não para treinamento, mas para controle. E esse mesmo controle é necessário não apenas para iniciantes, mas para a equipe como um todo.

Tudo isso é facilmente formulado na seguinte tese: " É necessária uma revisão de código para monitorar a conformidade com os acordos e regras gerais de todos os membros da equipe ". E o treinamento de iniciantes, neste caso, não passará de uma substituição artificial de uma meta que apenas confunde as pessoas e direciona o processo em uma direção diferente.

Mas mesmo com essa conclusão, que, ao que parece, foi formulada corretamente, a lenha pode ser quebrada. Uma revisão de código é uma fase que começa quando o código já está gravado. E pode acontecer que o código escrito sem seguir as regras gerais seja muito caro para personalizar o padrão geral. Nossa tarefa é informar ao contratado que algo deu errado o mais cedo possível. E, portanto, argumento que às vezes as revisões de código não são o processo mais apropriado para monitorar a conformidade com as regras gerais. Em muitos casos, as verificações de conformidade podem e devem ser transferidas para estágios anteriores.

Por exemplo, você pode verificar a formatação do código nos ganchos do sistema de controle de versão (bem como o uso de classes seguras, disposições para a localização de arquivos nas pastas correspondentes, o uso de dependências externas e bibliotecas de terceiros, etc.). Isso minimiza o tempo necessário para a revisão do código.

Continuando a conversa sobre o treinamento de iniciantes no processo de revisão de código, vamos fazer uma analogia. Suponha que você produza algum tipo de produto, por exemplo, bolos. Suponha que você tenha recebido um pedido de bolo para um casamento VIP. Você confia a “revisão” do bolo pronto ao novato? Você está pronto para o novo confeiteiro assumir a responsabilidade pela vergonha ou pelo sucesso de toda a padaria deste casamento? Ou talvez você queira que ele, nesta fase, se familiarize com as regras adotadas pela equipe (como assar bolos, preparar creme e insistir em cranberries com conhaque)? Obviamente, tanto o processo de introdução das regras para o iniciante quanto o controle da parte dele são coisas bastante estranhas nesse estágio: o bolo já foi assado. Então, por que podemos agir de maneira diferente com os recursos e o código? Ou os recursos que lançamos não trazem vergonha ou o sucesso de nossa equipe aos olhos de usuários e clientes?

Você pode contestar, dizendo que não preparamos tarefas “para pessoas importantes” todos os dias e que é inteiramente possível que um iniciante seja encarregado de uma tarefa não muito urgente ou não muito importante. E você estará certo.

Mas, primeiro, o que um iniciante aprende com tarefas triviais nas quais há poucas alterações de código (e essas mudanças não estão em locais críticos e para os negócios provavelmente não são tão importantes, pois a tarefa é urgente)? Como ele pode aprender a assar bolos se chicoteia o creme o tempo todo? A transição de simples para complexa, é claro, é necessária. Mas é necessário fazer isso, controlando cuidadosamente o processo. Iniciantes precisam ser ensinados, não deixar de aprender por conta própria.

E segundo, mesmo uma abordagem aparentemente útil e correta pode prejudicar uma empresa. Entender isso é muito simples, usando o método de levar ao ponto do absurdo para tornar a conclusão o mais simples e compreensível possível.

Imagine que declaramos abertamente que as tarefas que o gerente de produto nos dá são necessárias para o treinamento de recém-chegados. Porque E o mesmo da revisão de código: afinal, confiamos algumas tarefas simples e não urgentes aos iniciantes, para que eles aprendam a trabalhar da maneira habitual conosco.

O que isso pode resultar no final? Um artista zeloso que fielmente faz seu trabalho e acredita que tudo o que faz deve ser feito da melhor maneira possível, assumirá o processo de aprendizado. Ele pode definir a tarefa de fazer várias implementações em vez de uma, para que mais tarde possa mostrar ao aluno as desvantagens e vantagens de diferentes abordagens. Ele pode dar palestras, comparando as abordagens e as melhores práticas usadas na empresa e além. Ele pode fazer muito mais para educar o iniciante. Como resultado, o tempo necessário para concluir a tarefa aumentará, porque, em vez de desenvolver, nos concentramos no treinamento .

No caso de uma revisão de código, um funcionário tão zeloso inicia uma longa discussão nos comentários à revisão sobre os benefícios e perigos de determinadas implementações, publica trechos de código no Stack Overflow, mostrando que existem outras idéias em outras cabeças, fornece links para artigos e livros que o aluno deve leia para entender que sua implementação é imperfeita.

Esse é um processo de aprendizado normal que pode existir, mas que deve ser feito corretamente. E nem sempre vale a pena integrá-lo ao processo de solução de um problema comercial. Porque esses são dois processos diferentes. Deixe o iniciante fazer os diferentes componentes do bolo, faça várias opções, deixe que algo estrague e jogue fora. Deixe alguém mais experiente mostrar a ele como agir e recontar receitas antigas. Mas aprender “na linha de produção” que produz seu produto para os clientes não é uma boa ideia. E se você já decidiu isso, “conduza o recém-chegado” pela manivela, não permitindo que ele interfira no processo de produção durante o treinamento.

Se sua empresa não é uma instituição, uma escola, uma escola técnica ou qualquer outra instituição educacional, o treinamento não é de sua responsabilidade direta. A empresa contratou você para resolver outros problemas e obter outros resultados.

A propósito, é por isso que não adicionamos funcionalidade ao Codeisok para fazer upload de imagens, digitar e destacar o código nos comentários, embora tenha havido e ainda haja muitos pedidos para esses recursos. Promovemos a ideia de que a revisão de código não é um blog pessoal, nem um messenger e nem um serviço de treinamento. Uma ferramenta é necessária para outra. De fato, para indicar as falhas no código, um comentário em texto simples é mais que suficiente.

Revisão de código como uma nova visão do código


Vamos seguir em frente. O seguinte cenário de “troca de experiências” é o seguinte: fazemos algo na equipe, observando acordos internos, nossos olhos ficam embaçados e então uma nova pessoa chega (de outra empresa ou de outro componente) - e talvez ele veja falhas em nossa organização do trabalho.

A ideia é boa, parece razoável. De fato, e se estamos fazendo algo errado?

Mais uma vez, voltemos à vida da tarefa e ao início da fase de revisão de código. É tarde demais? O bolo já está assado, os bolos são manchados com creme, as rosas são coladas. O preço do erro é muito alto. E então descobrimos que em outra padaria nos bolos adicione refrigerante, cortado com suco de limão, para esplendor. E daí? O que fazer Remodelar?

Obviamente não, porque: 1) sempre ficamos sem refrigerante, e o resultado não foi ruim; 2) é possível que os clientes tirem bolos de nós, e não de outra padaria, porque não gostam do sabor do refrigerante; 3) o bolo está pronto, o casamento é hoje à noite e não teremos tempo para fazer um novo bolo.

Então por que isso é tudo? É necessário compartilhar experiências. É necessário um novo visual. Mas, parece-me, em um estágio diferente. Por exemplo, antes de assar bolos, você pode verificar com um iniciante: “E como você fez isso antes? Por que esse método é melhor? ” E, provavelmente, vale a pena fazer alguns bolos da maneira antiga e nova para dar a nossos clientes regulares uma chance de obter sua opinião.

A implementação impensada das melhores práticas vistas em artigos ou relatórios pode prejudicar a empresa e o produto. “O refrigerante é adicionado aos bolos por todos os principais players:“ Bugle ”,“ Tracebook ”,“ Rile.ru ”,“ Yumdeks ”. Todo mundo faz isso. E daí? Por causa disso, Petya deve enfrentar imediatamente um remake de uma tarefa que já está pronta?

Tenho certeza que não. Se, inicialmente, antes de resolver o problema, você não concordou que "haverá refrigerante nos bolos", é muito míope exigir sua adição no estágio de revisão do código. Sua empresa não tem o objetivo de "tomar refrigerante em bolos". Se seus bolos já estão vendendo bem, não importa se eles têm refrigerante ou não.

Você precisa entender que a troca de experiências é outro processo que não está vinculado a nenhuma revisão de código. Pode ocorrer mais cedo, mais tarde ou em paralelo, mas é diferente. Você pode organizar master classes mútuas com os concorrentes. Alguns estão tentando roubar uma receita super secreta e uma técnica ultramoderna de chantilly com um perfurador. Alguém está tentando espionar as idéias de outras pessoas e melhorar seus processos com a ajuda deles. Mas é estranho fazer isso em uma esteira de trabalho, no estágio em que seu produto está quase pronto, e o preço da conversão é muito alto.

Afinal, estamos falando sobre a fase de revisão. Uma revisão de código que já foi gravada e está pronta para a próxima etapa. Este código está aguardando o revisor clicar no botão "O código está OK". E seus clientes não estão preparados para o fato de que você preparará um bolo com um novo chef para mostrar a ele como você assa bolos e ouve suas críticas.

Revisão de código como uma introdução a um pedaço de código


Talvez isso pareça a parte mais razoável e compreensível, com a qual muitos concordam. O cenário aqui é entendido da seguinte maneira: um programador escreveu o código da tarefa, o resto olhou para ele, o estudou e agora eles sabem como trabalhar com ele. Assim, reduzimos o risco de um fator de barramento : se o desenvolvedor sair, outros poderão trabalhar com sucesso com seu código. E também estamos prontos para o fato de que na próxima vez que esse código puder ser "tocado" por outra pessoa, nesse caso, ele já saberá como trabalhar com ele.

Vamos pensar se isso realmente funciona como planejado e se realmente ajuda as pessoas a compartilhar competências e conhecimentos sobre seções específicas do código.


Vamos olhar a situação através dos olhos da pessoa que escreveu o código. , ?

, - (, , , ). , ? , . , , , .

, , , , . . , ?

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

, . , - ? — . , , .

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

? , , — . .

best practices « -» « , ». E daí? Best practices , , . , ? . , - , .

best practices, , . . — - , / — . « », « », « — , DI». ? ?

, , . , , — , .

, -, «», , - . ? - , , , . , . , .

-, « » «» , . -, . , , - , : ? . , , ( : , ).

, , . , , ( , ?).

— . , , . . ? bus factor , , - , .

, , , : , ?

— , , ?
— .
— ?
— .

. , ? , , , ? , , , — ?

, , , , , ?

, , , . , , .

, . , , , ( ), , ( , , , — , . .). , .

— , , , . ( , , . .) . — — .

Code review


« ?» . , , .

: - , , - , , . git blame , , , , , .

, , — , , , ( ) — . , , . . , ?


. ?

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

, , . ? , .

, , , . , . , ? .

, , — , . ( )?

« - - » . , . .

: . .


, , . , . , .

, , — . , .

— . , . , . , .

, , :


? , , ? ? , , ? - , ? ? ? E assim por diante

, . , . , .

, , . , , , . , , , .

, best practices, . , , , , . . , , . , .

,


. , , . - , , - .

, , . , API- . , bus factor, .

, . .


- ? , . , . null , « ». .

, . , ? ?

-


Esse estágio de verificação também é muito importante, pois visa melhorar o notório suporte ao código.

Muitas pessoas entendem o significado do termo manutenção de código, mas nem todos podem explicar o que é. E se sim, como definir a tarefa para a equipe para manter essa manutenibilidade? Portanto, acredito que um desenvolvedor que pensa em como seu código será testado, muito menos em testar seu próprio código e escrever um autoteste para automatizar o teste, naturalmente se esforçará para garantir que seu código atenda à maioria dos critérios de manutenção de código. De fato, sem isso, escrever um autoteste é quase impossível.

A recomendação geral para qualquer alteração de código também pode ser a decomposição correta das tarefas . Quanto menor a quantidade de código que passa pelo pipeline de todos os processos, da ideia à produção, mais fácil é esse código para verificar a conformidade, testar, combinar com o código de outras tarefas e fazer upload. Uma revisão de código não é exceção. Uma tarefa com um grande número de linhas alteradas em muitos arquivos é muito difícil de ler e entender (e manter as dependências em mente). O risco e o custo da correção de bugs podem ser muito altos.

Conclusão


Isso, de fato, é tudo. Esses quatro pontos são precisamente as mesmas coisas que precisam ser verificadas na fase de revisão. Eles são fáceis de formalizar, fáceis de transmitir aos artistas, o que significa que não é difícil formular critérios de verificação e prever a duração. A automação e outras maneiras de minimizar o tempo de revisão são mais do que possíveis.

E, finalmente, darei mais algumas dicas.

É importante lembrar que qualquer retorno do código para revisão não afeta a qualidade da implementação da melhor maneira. Mesmo que todos sejam bem-intencionados. No caso de revisão de código, funciona da seguinte maneira: o desenvolvedor corrige o código, mas não o testa tão completamente quanto na primeira iteração (por quê? Afinal, ele já verificou tudo e o modificou um pouco). A experiência mostrou que a maioria dessas situações se transforma em bugs precisamente nos locais em que houve correções com base nos resultados da revisão do código. É assim que a psicologia humana funciona.

No início do artigo, sugeri que você respondesse três perguntas sobre a necessidade e a correção de uma revisão. Agora você tem um algoritmo para ajudá-lo a encontrar as respostas. Conhecendo as etapas de controle e levando em consideração o tamanho da tarefa, é bem possível planejar o tempo necessário para a revisão do código, sempre tentando reduzi-la cada vez mais.

Ao implementar qualquer processo, é importante lembrar os objetivos que estabelecemos para nós mesmos e os problemas que queremos resolver. Então será muito mais fácil separar os grãos do joio e formular critérios mensuráveis ​​para avaliar a eficácia. E você precisa formulá-los para você e sua equipe, que também precisam resolver urgentemente os problemas que surgiram, mas que podem entendê-los à sua maneira.

Há mais um ponto importante: os processos, regras e valores que são justos para uma empresa podem não ser tão úteis e funcionar em outra. O que descrevi como exemplos da revisão correta funciona em nossa empresa. Promovemos o desenvolvimento rápido, lançamentos frequentes e a revisão de cada tarefa. Pense em como isso é aplicável ao seu projeto e que uma revisão é um daqueles processos que não podem ser deixados ao acaso.

Mas a decisão, é claro, permanece com você.

Source: https://habr.com/ru/post/pt413965/


All Articles