Guia para dar e receber um bom code review

Artigos - 26/Fev/2020 - por Campus Code

Ao longo dos anos adotamos uma série de práticas que se tornaram parte da cultura da Campus Code e gostaríamos de compartilhá-las. Para dar início a essa série de artigos, vamos começar falando sobre code review!

O code review (revisão de código) é uma ótima ferramenta para encontrar problemas que não são identificados na fase de testes. Grande parte das decisões, opiniões, práticas, etc. no desenvolvimento de software são ideias de uma pessoa ou um grupo.

Ao receber ou dar feedback sobre o código, foque nas qualidades e procure resolver os possíveis pontos negativos, oferecendo sugestões para resolução ao invés de críticas.

"Critique o código, não pessoas!". Com isso em mente inicie seu code review.

Sempre sugira, não comande. Prefira frases como:

O que acha de extrair um método aqui? Minha opinião é que esse nome poderia ser mudado.

Não use frases como:

Faça tal coisa!

Regras gerais

Procure ser o mais claro possível nos comentários e dúvidas. Não entendeu? Peça esclarecimento.

Não entendi. Poderia explicar de outra forma?

Se emojis facilitarem a comunicação, você pode utilizar, mas não exagere. ;)

Se estiver com dificuldades para entender os comentários, talvez seja melhor conversar por outro meio (chat, vídeo ou pessoalmente). Posteriormente, adicione um comentário com o resumo das soluções/sugestões feitas fora do code review e quais foram aplicadas ao código.

Recebendo review

Seja amável. Agradeça as sugestões:

Boa, não tinha percebido. Não sabia disso, vou dar uma olhada. Obrigado.

Antes de marcar um comentário como resolvido, explique o que foi feito:

Boa! Alterei o nome do método de get_name para name

Se uma sugestão foi feita e aplicada a um novo commit, opte por adicionar no comentário a hash do commit

Feito! Alterei o nome da variável para name (a928c9e)

Quando houverem muitas sugestões para o mesmo problema, informe qual foi aplicada e dê crédito para quem fez a sugestão:

Utilizei a sugestão de @fulano. Acredito que tenha melhorado a legibilidade e o código ficou melhor, valeu.

Adicione links que facilitem a compreensão do problema ou da solução. Exemplo: descrição da história, link do artigo que usou para escrever a solução, documentação, etc. Cuidado com a quantidade de links, adicione somente os que forem realmente necessários

Quando for criar o seu merge request, descreva as possíveis melhorias que ficaram pendentes (aquelas que não podem ser aplicadas no momento) e coloque-as em issues, histórias, backlog, etc.

Durante a revisão, opte por commits separados ao invés de amends. Isso facilita a legibilidade e rastreio do review. Faça squash antes de fazer o merge.

Mantenha sempre a branch atualizada com a master. Assim, evita-se a resolução de conflitos durante o merge.

Antes de solicitar o review, faça você mesmo um review do seu código e garanta que todos os testes estão passando na sua máquina e em CI. Também corrija a formatação do código para o Style Guide definido pela equipe.

Fazendo review

Tão importante quanto sugerir melhorias ou indicar possíveis problemas no código, é elogiar uma solução. Se você aprendeu algo ou percebeu o quão boa foi uma solução, deixe que o autor saiba disso.

Ao sugerir uma melhoria, opte por mostrar um código de exemplo ou um link, para que fique o mais claro possível.

Acho que nesse trecho você poderia usar guard clause. O que acha? Por exemplo

def dashboard
  return redirect_to dashboard_path(current_user) if current_user.admin?

   flash[:error] = "You are not an admin"
   redirect_to root_path
 end

Você pode dar uma olhada nesse post também.

Ao fazer sugestões, deixe claro quando é uma melhoria, uma solução alternativa ou, simplesmente, gosto pessoal. Tente apontar os prós e contras em relação à implementação:

Acho que poderia extrair para uma nova classe. Apesar de a classe ser pequena, não ferimos o Single Responsibility Principle

Ao sugerir uma alternativa, deixe claro que não é necessário seguir a sugestão. O mesmo vale para gosto pessoal:

Como o método é pequeno, eu prefiro utilizar o guard clause. O que acha? Não tem problema se ficar assim :)

Se as discussões se aprofundarem ou se prolongarem demais, talvez seja melhor movê-las para outro lugar (issue, forum, chat, call, card, após a daily, etc.).

Uma excelente palestra sobre o tema é da Elaine Naomi no GuruSP, onde ela fala sobre as motivações de fazer code review, como evitar comportamentos tóxicos e formas de tratar e avaliar opções para não impactar as entregas de software.

Outras referências:

Campus Code