Como conduzir uma revisão de código

Da documentação de práticas de engenharia do Google

Este guia fornece práticas recomendadas para a realização de revisões de código com base em anos de experiência. Juntos, eles compõem um documento, dividido em várias seções. Não é necessário ler todos eles, mas muitas vezes é melhor para você e a equipe estudar o manual na íntegra.


Consulte também o Guia do autor de CL para obter conselhos detalhados sobre desenvolvedores cujos commits são revisados.

Padrão de Revisão de Código


O principal objetivo de uma revisão de código é garantir a melhoria contínua da base de código do Google. Todas as ferramentas e processos são dedicados a esse objetivo.

São necessários vários compromissos aqui.

Primeiro, os desenvolvedores devem ser capazes de resolver com êxito seus problemas . Se você nunca enviar um código, a base de códigos nunca melhorará. Além disso, se o revisor complicar bastante qualquer trabalho, no futuro, os desenvolvedores não estarão interessados ​​em propor melhorias.

Por outro lado, é de responsabilidade do revisor garantir que a qualidade do CL não reduza a qualidade geral da base de código ao longo do tempo. Isso pode ser difícil, porque muitas vezes a degradação ocorre devido a uma ligeira diminuição na qualidade do código ao longo do tempo, especialmente se a equipe estiver sob forte pressão dos prazos e sentir que tem o direito de aumentar a dívida técnica.

Além disso, o revisor é responsável pelo código que está sendo revisado. Ele quer garantir que a base de código permaneça consistente, suportada e corresponda a tudo o mais mencionado na seção "O que fazer check-in de código" .

Assim, obtemos a seguinte regra como padrão para revisão de código:

Normalmente, os revisores devem endossar o CL assim que ele atinge um estado em que definitivamente melhora a qualidade geral do código do sistema, mesmo que o CL não seja perfeito.

Esta é a principal revisão de código entre todos os princípios.

Claro, ele tem limitações. Por exemplo, se o CL adicionar uma função que o revisor não deseja ver no sistema, certamente poderá recusar-se a confirmar, mesmo se o código for de boa qualidade.

O ponto principal aqui é que não há código "perfeito" - há apenas um código melhor . O revisor não deve exigir que o autor polonês cada fragmento minúsculo. Em vez disso, o revisor deve equilibrar a necessidade de mais progresso sobre a importância das mudanças propostas. Em vez de buscar o ideal, o revisor deve se esforçar para melhorar continuamente . Uma confirmação, que geralmente melhora a capacidade de manutenção, legibilidade e compreensão do sistema, não pode ser adiada por dias ou semanas porque não é "perfeita".

Os revisores sempre podem deixar comentários sobre como melhorar o código, mas alterações não muito importantes devem ser marcadas, por exemplo, com o prefixo Nit: para que o autor saiba que esse é apenas um ponto de vista que ele pode ignorar.

Nota Nada neste documento justifica CLs que degradam definitivamente a qualidade geral do código do sistema. Isso só é possível em caso de emergência .

Mentoring


Uma revisão de código também pode ser importante para ensinar aos desenvolvedores algo novo sobre a linguagem, estrutura ou princípios gerais de design de software. É sempre bom deixar comentários que ajudem o desenvolvedor a aprender algo novo. Compartilhar conhecimento contribui para melhorar o código do sistema ao longo do tempo. Lembre-se de que, se você deixar um comentário puramente educacional que não é crítico para atender aos padrões descritos aqui, adicione o prefixo Nit a ele : ou indique que o autor não é obrigado a permitir isso.

Princípios


  • Fatos e dados técnicos superam opiniões e preferências pessoais.
  • Em questões de estilo, a autoridade absoluta é o guia do estilo . Qualquer detalhe puramente estilístico (espaço etc.) que não esteja incluído no guia de estilo é uma questão de preferência pessoal. O estilo deve corresponder ao que é. Se não houver um estilo anterior, aceite o autor.
  • Aspectos do design de software quase nunca são uma questão de puro estilo ou preferência pessoal. Eles são baseados em princípios fundamentais e devem ser determinados por esses princípios, e não apenas pela opinião pessoal. Às vezes, existem várias opções válidas. Se o autor puder demonstrar (usando dados ou com base em princípios sólidos de engenharia) que certas abordagens são igualmente eficazes, o revisor deve aceitar a preferência do autor. Caso contrário, a escolha é ditada pelos princípios de desenvolvimento padrão.
  • Se nenhuma outra regra for aplicável, o revisor poderá solicitar ao autor que observe a uniformidade com a base de código atual, se isso não piorar as condições gerais do sistema.

Resolução de Conflitos


Em qualquer conflito, o primeiro passo deve ser sempre o desejo do desenvolvedor e do revisor de chegar a um consenso com base no conteúdo deste documento e de outros documentos no Guia do Autor de CL e neste Guia do Revisor .

Quando é especialmente difícil chegar a um consenso, uma reunião pessoal ou uma videoconferência entre o revisor e o autor pode ajudar (se você o fizer, certifique-se de anotar os resultados da discussão em um comentário ao comitê para futuros leitores).

Se isso não resolver a situação, a maneira mais comum é a escalação. Muitas vezes, consiste em uma discussão mais ampla com a equipe, atraindo o líder da equipe, entrando em contato com o mantenedor ou gerente de desenvolvimento. Não deixe o commit demorar devido ao fato de que o autor e o revisor não podem chegar a um acordo.

O que verificar no código


Nota Ao considerar cada um desses itens, certifique-se de considerar a Revisão de código padrão .

Desenho


O mais importante é considerar o projeto geral (design) na revisão de código. As interações entre diferentes partes do código fazem sentido? Essa alteração se aplica à sua base de código ou biblioteca? O CL se integra bem ao resto do sistema? Está na hora de adicionar essa funcionalidade agora?

Funcionalidade


Esse CL faz o que o desenvolvedor pretendia? É bom para os usuários desse código? Por "usuários", entendemos os usuários finais (se eles são afetados pela mudança) e os desenvolvedores (que terão que "usar" esse código no futuro).

Basicamente, esperamos que, mesmo antes de confirmar, os desenvolvedores testem seu código suficientemente bem para que funcione corretamente. Mas, como revisor, você ainda precisa pensar em casos extremos, procurar problemas de simultaneidade, tentar pensar como usuário e até assistir o código ao ler que não há erros óbvios.

Se você quiser, pode verificar o desempenho. Isso é mais importante se o código afetar os usuários, como alterar a interface do usuário . É difícil entender como algumas mudanças afetarão os usuários quando você acabou de ler o código. Para essas alterações, você pode solicitar ao desenvolvedor que forneça uma demonstração se for muito difícil se aprofundar no código e tentar você mesmo.

Outro ponto em que é especialmente importante pensar na funcionalidade durante uma revisão de código é se ocorrer algum tipo de programação paralela no CL, que teoricamente pode causar conflitos ou condições de corrida. Esses problemas são muito difíceis de detectar simplesmente executando o código; geralmente é necessário que alguém (tanto o desenvolvedor quanto o revisor) pense cuidadosamente sobre eles e verifique se não há problemas introduzidos (observe que esse também é um bom motivo para não usar modelos de paralelismo nos quais condições de corrida ou impasses são possíveis - isso pode ser feito por código muito difícil de entender ou revisão de código).

Dificuldade


CL é mais complicado do que deveria ser? Verifique em todos os níveis: linhas, funções e classes separadas. “Complexidade excessiva” geralmente significa a incapacidade de entender rapidamente durante a leitura . Também pode significar que é mais provável que os desenvolvedores introduzam erros ao tentar chamar ou modificar esse código .

Um tipo especial de complexidade é o excesso de engenharia, quando os desenvolvedores tornaram o código mais universal do que deveria, ou adicionaram funcionalidades que o sistema não precisa atualmente. Os revisores devem estar especialmente vigilantes com o excesso de engenharia. Incentive os desenvolvedores a resolver um problema que definitivamente deve ser resolvido agora, em vez de um problema que talvez precise ser resolvido no futuro. Um problema futuro deve ser resolvido quando ele aparecer e você poderá ver sua forma e requisitos reais no universo físico.

Testes


Solicite testes de unidade, integração ou de ponta a ponta que sejam relevantes para a mudança. Em geral, os testes devem ser adicionados ao mesmo CL que o código de produção se o CL não atender à emergência .

Verifique se os testes estão corretos, razoáveis ​​e úteis. Os testes não se testam e raramente escrevemos testes para os nossos testes - uma pessoa deve garantir que os testes sejam válidos.

Os testes realmente falham em código quebrado? Se o código mudar, haverá falsos positivos? Cada teste faz declarações simples e úteis? Os testes estão divididos corretamente entre diferentes métodos de teste?

Lembre-se de que os testes são códigos que também precisam ser suportados. Não permita complexidade neles apenas porque não faz parte do arquivo binário principal.

Nomeação


O desenvolvedor escolheu bons nomes em todos os lugares? Um bom nome é longo o suficiente para transmitir completamente o que é o elemento ou o que ele faz, sem ser tão longo que se torne difícil de ler.

Comentários


O desenvolvedor escreveu comentários claros em linguagem simples? Todos os comentários são necessários? Os comentários geralmente são úteis quando explicam por que existe algum código e não devem explicar o que esse código faz. Se o código não for suficientemente claro para se explicar, deve ser simplificado. Existem algumas exceções (por exemplo, comentários explicando as ações do código geralmente são muito úteis para expressões regulares e algoritmos complexos), mas a maioria dos comentários é destinada a informações que o próprio código não pode conter, por exemplo, para fundamentar uma decisão.

Também pode ser útil examinar os comentários no código anterior. Talvez haja um TODO, que agora pode ser excluído, ou um comentário que não recomende a introdução dessa alteração, etc.

Observe que os comentários diferem da documentação de classes, módulos ou funções, que descreve a tarefa do código, como ele deve ser usado e como ele se comporta.

Estilo


Temos guias de estilo do Google para todos os principais idiomas e até para a maioria dos menores. Verifique se o CL não está em conflito com os guias de estilo relevantes.

Se você deseja melhorar alguns elementos que não estão no guia de estilo, adicione uma nota ao comentário ( Nit :) . O desenvolvedor saberá que esta é sua observação pessoal, que não é vinculativa. Não bloqueie o envio de confirmações com base apenas em suas preferências pessoais.

O autor não deve combinar alterações significativas de estilo com outras alterações. Isso dificulta a visualização de alterações no CL, complica mesclagens, reversões de código e causa outros problemas. Por exemplo, se o autor quiser reformatar o arquivo inteiro, solicite uma alteração de estilo em um CL e envie o CL com alterações funcionais.

A documentação


Se a saída do CL alterar algo na montagem, teste, procedimentos de interação ou liberação de código, verifique se há atualizações na documentação relevante, incluindo arquivos README, páginas g3doc e todos os documentos de referência gerados. Se o CL remover o código ou torná-lo obsoleto, considere também remover a documentação. Se a documentação estiver faltando, solicite sua criação.

Cada linha


Veja cada linha no código. Embora quaisquer arquivos de dados, código gerado ou grandes estruturas de dados possam ser revisados ​​brevemente, mas leia cuidadosamente todas as classes, funções ou blocos de códigos escritos por uma pessoa, nunca assuma por padrão que tudo está em ordem. Obviamente, algum código merece um estudo mais completo que outro - você decide por si mesmo, mas deve pelo menos ter certeza de que entende a operação de todo o código.

Se for muito difícil para você ler o código e isso atrasar a revisão, informe o desenvolvedor e aguarde até que ele traga clareza antes de continuar. No Google, contratamos ótimos engenheiros de software e você é um deles. Se você não conseguir entender o código, é muito provável que outros desenvolvedores não consigam. Dessa forma, você também ajuda futuros desenvolvedores a entender esse código ao pedir esclarecimentos ao desenvolvedor.

Se o código for compreensível, mas você não se sentir qualificado o suficiente para avaliar um fragmento, verifique se o CL possui um revisor qualificado, especialmente para problemas complexos, como segurança, concorrência, acessibilidade, internacionalização etc.

Contexto


Muitas vezes, é útil olhar para CL em um contexto amplo. Normalmente, uma ferramenta de revisão de código mostra apenas algumas linhas perto do local da mudança. Às vezes, você precisa examinar o arquivo inteiro para garantir que a alteração realmente faça sentido. Por exemplo, você vê a adição de apenas quatro linhas, mas se você olhar o arquivo inteiro, essas quatro linhas estão no método de 50 linhas, que agora realmente precisam ser divididas em linhas menores.

Também é útil pensar sobre CL no contexto do sistema como um todo. Melhora a qualidade do código do sistema ou o torna mais complexo, menos testado etc.? Não aceite confirmações que degradam o código do sistema. A maioria dos sistemas é complicada pela soma de muitas pequenas alterações, por isso é importante evitar dificuldades menores lá.

Bom


Se você vir algo bom no commit, informe o desenvolvedor, especialmente quando ele tiver resolvido o problema descrito em um de seus comentários da melhor maneira possível. As revisões de código geralmente se concentram nos erros, mas também devem incentivar e valorizar as boas práticas. Do ponto de vista da orientação, às vezes é ainda mais importante dizer ao desenvolvedor o que exatamente ele fez certo, e não onde ele cometeu um erro.

Sumário


Ao executar uma revisão de código, verifique se:

  • O código está bem desenhado.
  • A funcionalidade é boa para usuários de código.
  • Quaisquer alterações na interface do usuário são razoáveis ​​e com boa aparência.
  • Qualquer programação simultânea é segura.
  • O código não é mais complicado do que deveria ser.
  • O desenvolvedor não implementa o que pode ser necessário no futuro com perspectivas desconhecidas.
  • O código é alinhado com testes de unidade apropriados.
  • Os testes são bem projetados.
  • O desenvolvedor usou nomes claros em todos os lugares.
  • Os comentários são compreensíveis e úteis, e basicamente respondem à pergunta por quê? mas não o que?
  • O código está devidamente documentado (geralmente no g3doc).
  • O código está em conformidade com nossos guias de estilo.

Durante a revisão, verifique cada linha de código, o contexto , verifique se você está melhorando a qualidade do código e elogie os desenvolvedores pelo bem que eles conseguiram fazer.

Navegação CL na revisão de código


Sumário


Agora que você sabe o que fazer check-in de código , qual é a maneira mais eficiente de realizar revisões de código em vários arquivos?

  1. Essa mudança faz sentido? Ele tem uma boa descrição ?
  2. Primeiro, observe a parte mais importante. Está bem desenhado?
  3. Veja o restante do CL na sequência apropriada.

Etapa 1: dê uma olhada em todo o commit


Veja a descrição do CL e o que ele faz em geral. Essa mudança faz algum sentido? Se não deveria ter sido escrito inicialmente, responda imediatamente com uma explicação de por que não é necessário. Quando você rejeita essa alteração, também é bom oferecer ao desenvolvedor o que fazer.

Por exemplo, você pode dizer: "Parece que você fez um bom trabalho, obrigado!" No entanto, removeremos o sistema FooWidget, por isso não queremos fazer novas alterações no momento. Que tal você refatorar nossa nova classe BarWidget? ”

Observe que o revisor não apenas rejeitou o CL atual e forneceu uma sugestão alternativa, mas também o fez educadamente . Essa cortesia é importante porque queremos mostrar que nos respeitamos como desenvolvedores, mesmo quando discordamos um do outro.

Se você receber vários CLs indesejados, revise o processo de desenvolvimento em sua equipe ou regras publicadas para colaboradores externos para melhorar a comunicação ao escrever o CL. É melhor dizer às pessoas "não" antes que elas executem uma tonelada de trabalho que terá que ser jogado fora ou reescrito pesadamente.

Etapa 2: Aprenda as partes básicas do CL


Localize o arquivo ou arquivos que representam a parte "principal" deste CL. Geralmente, há um arquivo com as alterações mais lógicas e essa é a parte principal do CL. Primeiro, observe essas partes principais. Isso ajuda a entender o contexto de partes menores do CL e geralmente acelera a execução da revisão de código. Se o CL for muito grande, pergunte ao desenvolvedor o que ver primeiro ou peça para ele dividir o CL em partes .

Se você encontrar sérios problemas na parte principal do CL, envie esses comentários imediatamente, mesmo se não tiver tempo para visualizar o restante do CL agora. De fato, observar o restante do código pode ser uma perda de tempo, porque se os problemas forem significativos o suficiente, muitos dos outros trechos de código em questão desaparecerão e não serão importantes.

Há duas razões principais pelas quais é tão importante enviar esses comentários principais imediatamente:

  • Os desenvolvedores costumam enviar CL e, em seguida, iniciam imediatamente um novo trabalho com base nele, enquanto aguardam o resultado de uma revisão de código. Se o CL tiver problemas sérios, eles terão que refazer o próximo CL. É aconselhável identificar erros o mais cedo possível, antes que eles executem muito trabalho extra em cima do design problemático.
  • As principais alterações demoram mais que as pequenas edições. Quase todos os desenvolvedores têm prazos. Para manter dentro deles e não reduzir a qualidade do código, qualquer refatoração importante deve começar o mais rápido possível.

Etapa 3: Percorra o restante do CL na sequência apropriada.


Depois de verificar se não há problemas sérios no design do CL como um todo, tente descobrir a sequência lógica para exibir arquivos e verifique se nada está faltando. Geralmente, quando você olha para os arquivos principais, a maneira mais fácil é simplesmente analisar o restante na ordem em que a ferramenta de revisão de código os apresenta. Às vezes, também é útil ler os testes primeiro e depois o código principal, porque você terá uma idéia do significado da mudança.

Velocidade de revisão de código


Por que uma revisão de código deve ser feita rapidamente?


No Google, otimizamos a velocidade da colaboração , não a velocidade dos desenvolvedores individuais. A velocidade do trabalho individual é importante, simplesmente não é tão prioritária em comparação com a velocidade do trabalho em equipe.

Quando você olha lentamente para o código, várias coisas acontecem:

  • A velocidade da equipe como um todo diminui. Sim, se uma pessoa não responder rapidamente a uma revisão de código, ela fará outro trabalho. No entanto, para o restante da equipe, os novos recursos e as correções são adiados por dias, semanas ou meses, pois cada CL aguarda uma revisão de código e uma revisão de código repetida.
  • -. , , . , «» . (, ), , . - .
  • A qualidade do código pode sofrer. Retardar a revisão do código aumenta o risco de que os desenvolvedores não enviem CLs tão bons quanto poderiam. Revisões lentas também dificultam a limpeza do código, a refatoração e outras melhorias nos CLs existentes.

Como executar rapidamente uma revisão de código?


Se você não estiver no meio de uma tarefa focada, faça um código de revisão logo depois que ele chegar.

Um dia útil é o tempo máximo para uma resposta (ou seja, é a primeira coisa na manhã seguinte).

Seguir essas diretrizes significa que um CL típico deve receber várias rodadas de revisão (se necessário) dentro de um dia.

Velocidade e distração


Há um momento em que a prioridade da velocidade pessoal excede a velocidade da equipe. Se você estiver no meio de uma tarefa focada, como escrever código, não se distraia com a revisão do código. Estudos demonstraram que pode levar muito tempo para um desenvolvedor retornar a um fluxo de desenvolvimento suave após a distração. Portanto, a distração da codificação realmente custa à equipe mais do que pedir a outro desenvolvedor que espere um pouco pela revisão do código.

Em vez disso, aguarde um ponto de interrupção no seu trabalho. Por exemplo, após concluir a tarefa atual, após o almoço, retornando de uma reunião, retornando de uma cozinha americana do escritório, etc.

Respostas rápidas


Quando falamos em velocidade de revisão de código, estamos interessados ​​no tempo de resposta , e não em quanto tempo é necessário para todo o processo até o fim. Idealmente, todo o processo também deve ser rápido, mas é ainda mais importante que respostas individuais cheguem rapidamente do que todo o processo .

Mesmo que todo o processo de revisão de código consuma tempo, a disponibilidade de respostas rápidas do revisor ao longo do processo simplifica bastante a vida dos desenvolvedores que podem se incomodar com respostas "lentas".

Se você estiver ocupado demais para uma revisão completa imediatamente após receber a solicitação, ainda poderá responder rapidamente com uma mensagem sobre os prazos ou oferecer a revisão a outros revisores que possam responder mais rapidamente ouforneça alguns comentários gerais iniciais (observação: nada disso significa que você deve interromper a codificação mesmo para enviar essa resposta - envie a resposta em um ponto de interrupção razoável em seu trabalho).

É importante que os revisores gastem tempo suficiente revisando e certifique-se de que seu "LGTM" signifique "este código esteja em conformidade com nossos padrões ". No entanto, as respostas individuais devem, idealmente, ser rápidas de qualquer maneira .

Revisão de código entre fusos horários


Ao trabalhar entre diferentes fusos horários, tente responder ao autor enquanto ele ainda estiver no escritório. Se ele já foi para casa, tente responder antes de retornar ao escritório no dia seguinte.

LGTM reservado


Por motivos de velocidade, há certas situações nas quais o revisor deve conceder LGTM / aprovação, mesmo no caso de comentários não respondidos no CL. Isso é feito se:

  • O revisor está confiante de que o desenvolvedor considerará adequadamente todos os comentários restantes.
  • Outras alterações são pequenas e opcionais .

O revisor deve indicar qual dessas opções ele quer dizer, se isso não estiver claro.

LGTMs reservadas são especialmente úteis quando o desenvolvedor e o revisor estão em fusos horários diferentes; caso contrário, o desenvolvedor aguardará o dia todo para obter a “aprovação da LGTM”.

Large CL


Se alguém lhe enviar um código de revisão tão grande que você não tiver certeza de quando pode visualizá-lo, a resposta típica seria pedir ao desenvolvedor que dividisse o CL em vários CLs menores . Isso geralmente é possível e muito útil para os revisores, mesmo que exija trabalho adicional do desenvolvedor.

Se o CL não puder ser dividido em CLs menores e você não tiver tempo para analisar rapidamente tudo isso, pelo menos, escreva alguns comentários sobre o design geral do CL e envie-os de volta ao desenvolvedor para melhorias. Um de seus objetivos como revisor deve sempre ser "desbloquear" o desenvolvedor ou permitir que ele tome rapidamente outras ações sem sacrificar a qualidade do código.

Melhorias na revisão de código ao longo do tempo


Se você seguir estas recomendações e abordar estritamente a revisão de código, verá que todo o processo acelera e acelera com o tempo. Os desenvolvedores descobrirão o que é necessário para código de alta qualidade e enviarão CLs excelentes desde o início, exigindo cada vez menos tempo para visualização. Os revisores aprendem a responder rapidamente e a não adicionar atrasos desnecessários ao processo de revisão. Mas não comprometa os padrões de revisão de código ou a qualidade para uma melhoria de velocidade imaginária - na verdade, você não alcançará uma aceleração geral a longo prazo.

Situações de emergência


Também há situações de emergência em que os CLs precisam passar por todo o processo de revisão de código muito rapidamente e onde os princípios de qualidade terão que ser suavizados. Mas leia a descrição de quais situações se qualificam como emergência e quais não.

Como escrever comentários na revisão de código


Sumário


  • Seja educado.
  • Explique seu raciocínio.
  • Evite pedidos explícitos simplesmente apontando problemas e deixando o desenvolvedor decidir.
  • Incentive os desenvolvedores a simplificar o código ou adicionar comentários, em vez de explicações simples para a complexidade.

Polidez


Em geral, é importante ser educado e respeitoso, e também muito claro e útil para o desenvolvedor cujo código você está visualizando. Uma maneira de fazer isso é garantir que você sempre faça comentários sobre o código e nunca sobre o desenvolvedor . Você nem sempre precisa seguir essa prática, mas deve usá-la quando diz algo desagradável ou controverso. Por exemplo:

Ruim: "Por que você usou fluxos aqui se é óbvio que a concorrência não é benéfica?"

Bom: “O modelo de concorrência aqui adiciona complexidade ao sistema e não vejo nenhum benefício real de desempenho. Como não há vantagem de desempenho, é melhor que esse código seja de thread único em vez de usar vários threads. ”

Explique os motivos


No exemplo "bom" acima, você pode ver que isso ajuda o desenvolvedor a entender por que você está fazendo seu comentário. Você nem sempre precisa incluir essas informações em seus comentários, mas às vezes é apropriado fornecer um pouco mais de explicação sobre sua lógica, as melhores práticas que você segue ou como sua sugestão melhora a qualidade do código.

Instruções


Em geral, fazer correções é uma tarefa do desenvolvedor, não do revisor. Você não precisa desenvolver uma solução de rascunho detalhada ou escrever um código para o desenvolvedor.

No entanto, isso não significa que o revisor não possa ajudar aqui. Em geral, deve-se encontrar um equilíbrio adequado entre identificar problemas e fornecer assistência direta. Apontar problemas e dar ao desenvolvedor a oportunidade de tomar uma decisão geralmente ajuda o desenvolvedor a aprender e facilita a revisão do código. Também pode levar a uma solução melhor, porque o desenvolvedor está mais próximo do código do que o revisor.

No entanto, instruções diretas, sugestões ou mesmo código são algumas vezes mais úteis. O principal objetivo de uma revisão de código é obter o melhor CL possível. O objetivo secundário é melhorar as habilidades dos desenvolvedores para que a revisão leve cada vez menos tempo para eles.

Aceite a explicação


Se você pedir ao desenvolvedor que explique um pedaço de código incompreensível, geralmente isso levará ao fato de que ele será reescrito com mais clareza . Às vezes, adicionar um comentário também é uma resposta apropriada, se não for apenas uma explicação de código muito complexo.

Explicações escritas apenas na ferramenta de revisão não ajudarão futuros leitores. Eles são aceitáveis ​​apenas em alguns casos, por exemplo, quando você olha para uma área com a qual não está muito familiarizado, e o desenvolvedor explica o que os leitores de código comuns já devem saber.

Como superar a resistência no processo de revisão de código


Às vezes, um desenvolvedor discute em um processo de revisão de código. Ou ele não concorda com sua proposta ou reclama que você é muito rigoroso em geral.

Quem está certo?


Quando o desenvolvedor não concordar com sua proposta, primeiro reserve um tempo para considerar sua posição. Muitas vezes, ele está mais próximo do seu código e, portanto, pode realmente ter uma idéia melhor de alguns aspectos. Faz sentido em seu argumento? Faz sentido em termos de qualidade do código? Nesse caso, admita que ele está certo e a pergunta desaparecerá.

No entanto, os desenvolvedores nem sempre estão certos. Nesse caso, o revisor deve explicar melhor por que ele acredita que sua proposta está correta. Uma boa explicação demonstra um entendimento da resposta do desenvolvedor e informações adicionais sobre por que a alteração é necessária.

Em particular, quando o revisor considerar que sua proposta melhorará a qualidade do código, ele deve insistir se considerar que a melhoria resultante da qualidade justifica o trabalho adicional. Melhorar a qualidade do código, como regra, ocorre em pequenas etapas.

Às vezes, são necessárias várias rodadas de explicação antes de ser realmente aceito. Apenas certifique-se de permanecer sempre educado e informe ao desenvolvedor que você o ouve, apenas não concorde.

Sobre o ressentimento dos desenvolvedores


Às vezes, os revisores sentem que um desenvolvedor fica chateado se insistir em melhorar. Às vezes, os desenvolvedores ficam realmente chateados, mas geralmente não por muito tempo; depois, eles ficarão muito agradecidos por você ter ajudado a melhorar a qualidade do código. Geralmente, se você é educado em seus comentários, os desenvolvedores não ficam realmente chateados e a preocupação está apenas na cabeça do revisor. Geralmente, o estilo de escrever comentários é mais frustrante do que a persistência do revisor em relação à qualidade do código.

Edições pendentes


Uma fonte típica de controvérsia é que os desenvolvedores (por razões óbvias) querem fazer o trabalho. Eles não querem passar por outra rodada de análises para este CL. Portanto, eles dizem que removerão algo no CL posterior, e agora você deve criar o LGTM para esse CL. Alguns desenvolvedores fazem isso muito bem e escrevem imediatamente outro CL que resolverá o problema. No entanto, a experiência mostrou que quanto mais tempo decorrido após o CL original, menor a probabilidade de que essa edição ocorra. De fato, geralmente se o desenvolvedor não faz a edição imediatamenteisso geralmente nunca. Não porque os desenvolvedores são irresponsáveis, mas porque eles têm muito trabalho e a edição é perdida ou esquecida sob a rampa de outros trabalhos. Portanto, geralmente é melhor insistir em uma correção imediata antes que o commit entre na base de código e seja "concluído". Permitir edições pendentes é uma maneira típica de degenerar bases de código.

Se o CL introduzir uma nova complexidade, ele deverá ser corrigido antes do envio, se isso não for uma emergência. Se o CL mostrar outros problemas que não podem ser resolvidos no momento, o desenvolvedor deve registrar um bug no rastreador e atribuí-lo a si mesmo para que ele não se perca. Opcionalmente, ele pode escrever um comentário TODO no código referente a esse erro.

Queixas gerais de gravidade


Se você costumava realizar uma revisão de código bastante superficial e alternar para o modo estrito, alguns desenvolvedores começam a reclamar muito alto. O aumento da velocidade de revisão de código geralmente resolve essas reclamações.

Às vezes, pode levar meses para que as queixas desapareçam, mas, no final, os desenvolvedores tendem a ver o valor de análises rigorosas de código, porque veem quão grande é o código. Às vezes, os manifestantes mais barulhentos até se tornam seus defensores mais fortes quando algo acontece que os faz realmente ver o valor de críticas rigorosas.

Resolução de Conflitos


Se você estiver fazendo todas as ações acima, mas ainda encontrar conflitos, consulte a seção Padrão de revisão de código para obter recomendações e princípios que podem ajudar a resolver o conflito.

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


All Articles