Estou interessado nas perguntas de revisão de código há muito tempo. Muitas vezes, surgiu um ou outro problema, com a qualidade do código ou com o clima da equipe. De fato, a revisão de código é, se não o único, um dos lugares mais importantes para conflitos na equipe de desenvolvimento.
E, recentemente, em preparação para o próximo lançamento do podcast Zinc Prod , descobri que o Google publicou um código de prática de Revisão de Código repleto de pensamentos valiosos. Todo o material é bastante volumoso e não se encaixa em um artigo, então tentarei destacar os pensamentos mais interessantes (para mim).
Então vamos
Termos usados no artigo original:
CL - lista de alterações. Mas geralmente é chamado de Solicitação de mesclagem ou solicitação de recebimento, portanto, no artigo, usarei a abreviação MR
LGTM - Parece bom para mim. Em suma, quando eles pressionam o botão "aprovar". Vou usar o termo "aprov", mais compreensível para a população.
Ideality mr
Na prática, pode-se encontrar vários extremos ao considerar a RM. Alguém como um time inteiro comenta zadalivaet, até que tudo é trivial ao ponto, é corrigido, alguém olha a lógica e pressiona "upruv".
Um pensamento interessante está escrito no documento do Google. O MR não deve ser perfeito, mas deve melhorar a base de código. Ou seja, a cada alteração introduzida, o código deve ficar cada vez melhor. E se o MR agregar muito bem, você não precisará encontrar falhas nas pequenas coisas; é mais lucrativo obter essa melhoria mais rapidamente.
Nenhum MR deve piorar o código. A única exceção é se o MR for uma correção urgente para algo.
Liberdade para comentar
Apesar de você não conseguir se livrar das ninharias, no entanto, você pode escrevê-las livremente em pelo menos todas as linhas. A contradição com o parágrafo anterior é resolvida simplesmente: o revisor prefixa as ninharias e escolhe o nit "pref:" do inglês. nitpick (nitpicking). Não é necessário corrigir esses comentários, no entanto, o autor do MR pode querer corrigir algo ou, mesmo se não, levar em consideração alguns pontos para o futuro.
Fatos sobre preferências pessoais
Quase sempre, os princípios padrão de design de software, baseados nas melhores práticas, são melhores do que bicicletas complicadas. Portanto, deve ser dada preferência a eles.
Se você pode aplicar várias abordagens padrão, isso fica a critério do autor.
Orçamento direto para uma melhor compreensão:
Aspectos do design de software quase nunca são uma questão de estilo puro ou apenas uma preferência pessoal. Eles são baseados em princípios subjacentes e devem ser ponderados sobre esses princípios, não simplesmente pela opinião pessoal. Às vezes, existem algumas opções válidas. Se o autor puder demonstrar (por meio de dados ou com base em princípios sólidos de engenharia) que várias abordagens são igualmente válidas, o revisor deve aceitar a preferência do autor. Caso contrário, a escolha é ditada pelos princípios padrão do design de software.
Se o revisor e o autor do MR não concordarem entre si
Primeiro vem uma tentativa de alcançar consenso nos comentários sobre RM. Se isso não funcionar, então uma discussão pessoal. Se você ainda não chegar a um consenso, atraia os membros da equipe. Mas o mais importante é que a RM não deve ficar parada por muito tempo devido ao desacordo de duas pessoas.
Discutido em comentários -> Discutido pessoalmente -> Discutido em equipe -> Seguindo em frente
MR verificar velocidade
A velocidade é extremamente importante. Se você fizer um comentário a cada poucos dias, o autor do MR irá reclamar que ele encontrou alguma falha nele e impedi-lo de trabalhar.
Se o MR for verificado muito rapidamente, quaisquer comentários não serão um grande problema - afinal, suas correções serão verificadas aqui e a tarefa continuará.
O Google aconselha que você não se distraia de se concentrar em sua tarefa, mas se estiver distraído, verifique se há algo a ser revisado. Por exemplo, ele voltou do almoço - ele revisou. Chegou ao trabalho - revisado. Etc.
Ordem de visualização MR
Primeiro você precisa olhar para o MR como um todo. É necessário, o código está no lugar certo (ou deveria estar em uma biblioteca separada), há algum problema global.
I.e. não faz sentido olhar para alguns detalhes da implementação se o código não estiver lá e nem estiver.
Em geral, a ordem de visualização é importante para fornecer feedback ao autor o mais rápido possível (consulte o parágrafo anterior).
Quando você olhou para o MR como um todo, precisa examinar os arquivos principais, ou seja, verifique a coisa mais importante (se não estiver claro o que é mais importante, você pode perguntar ao desenvolvedor).
Novamente, qualquer problema deve ser relatado imediatamente, mesmo se você ainda não tiver inspecionado o RM e decidido inspecioná-lo em geral mais tarde.
Em seguida, você precisa selecionar uma sequência lógica para visualizar os arquivos restantes. Nenhum arquivo deve ser ignorado.
O que procurar ao visualizar
- O código está bem desenhado
- A funcionalidade é bem feita do ponto de vista dos usuários desse código, não importa quem eles sejam.
- A aparência (se houver) deve ser boa
- Todas as nuances da programação paralela (se houver) são levadas em consideração.
- Código não redesenhado
- O desenvolvedor não trabalha demais: você não precisa escrever o código que pode ser necessário ou pode não ser necessário
- O código tem testes
- Os testes são bem projetados
- Os nomes (para tudo) são bem selecionados
- Comentários sobre o código são compreensíveis e necessários. Eles devem explicar por que isso é feito, e não como é feito.
- Adicionada documentação.
- O código corresponde aos guias de estilo.
MR muito grande
MRs muito grandes devem ser solicitados para serem divididos. É quase sempre possível.
Como escrever comentários na revisão de código
- Você precisa ser educado, não pessoal. Discuta o código, não o codificador.
- Não basta emitir diretrizes para correções, mas explicar por que você precisa corrigi-las.
- Mantenha um equilíbrio: identifique o problema e empurre o desenvolvedor para que ele mesmo entenda a melhor forma de resolvê-lo; ou emita imediatamente uma solução pronta. O primeiro desenvolve o desenvolvedor (benefício estratégico), o segundo melhora e acelera o MR (benefício tático).
- Se o revisor não entendeu em algum momento do código e pede ao autor que explique o que é, então a melhor resposta seria alterar o código. De modo que tudo ficou claro a partir do código sem questionar.
Se você não concorda com sua opinião
É necessário entender em detalhes. Talvez você não entenda alguma coisa, peça mais informações. Talvez o oposto. De qualquer forma, muitas vezes o entendimento só ocorre após algumas rodadas de explicações dos dois lados.
Medo do autor perturbador MR
Acontece que o desenvolvedor fica chateado se o revisor insistir em algumas alterações. No entanto, na maioria das vezes os desenvolvedores ficam chateados por causa da forma do comentário, e não por causa do conteúdo. Seja educado, explique com argumentos e provavelmente tudo ficará bem.
"Eu vou consertar isso mais tarde."
Se o desenvolvedor concorda que há um problema no código, mas pede para corrigir o MR, prometendo corrigi-lo outra vez, então, de acordo com os funcionários do Google, esse "mais tarde" na maioria das vezes nunca ocorre. Portanto, se o MR não for uma correção de bug urgente, você precisará insistir em uma correção.
Conclusões
Gostei muito deste documento do Google. Especialmente o truque da vida com a palavra "nit", a ênfase na velocidade de revisão do código e também que o MR não deve ser perfeito. Parece ser simples, mas enquanto isso claramente não for considerado, não chega ao ponto.
Se este artigo parecer interessante para os leitores e oferecer várias vantagens, escreverei a seguinte parte: o processo de revisão de código pelo autor MR.
Atualização: a segunda parte do artigo aqui
Além disso, na próxima edição da Zinc Sale, discutiremos em detalhes o código de revisão de diferentes ângulos. Portanto, assine nosso podcast de desenvolvimento!