Gabriel / Ramos

⟨ pintor de pixel ⟩

  • Blog
  • Chegou a hora da revisão

Chegou a hora da revisão

Algumas abordagens que me ajudam tecnicamente e facilitam o relacionamento com o time na hora de revisar código

6 min. de leitura

Foto por Markus Winkler

Revisão de código (e os famosos PRs ou pull-requests) é um assunto bem particular e faz parte da vida de quase todo mundo que programa.

Cada pessoa tem sua forma de indicar pontos positivos ou negativos quando está revisando um código e ao longo dos últimos tempos notei que muitos desses pontos eu acabo fazendo naturalmente. A grande maioria deles, com certeza, contribuiu pra boas discussões técnicas onde eu pude aprender muito e também estreitar relações.

Abaixo eu listei em tópicos algumas das abordagens que eu, particularmente, gosto de adotar quando estou revisando algum código (ou tendo o meu código revisado).

Não assuma que todo mundo tem o contexto do seu código

Embora nem todo mundo leve em consideração, seu PR começa com um bom título e descrição.

Perder alguns segundos detalhando minimamente como aquela mudança afeta o projeto ou pode ser testada ajuda muito a compartilhar o conhecimento daquele projeto, afinal, um PR não deixa de ser uma pequena documentação também.

Alguns tópicos e perguntas que podem rapidamente direcionar a descrição do seu PR:

  • Objetivo: qual mudança/ajuste seu PR está realizando?
  • Como testar: é necessário executar o projeto localmente ou em algum ambiente interno da sua empresa?
  • Resultado esperado: após testar, como posso garantir que esse objetivo foi concluído?

Se fossemos escrever alguma descrição de um PR com isso, poderíamos pensar em algo como:

# Titulo
Opção de cancelar na modal de cadastro de usuários

# Descrição
### Objetivo
Ajustar modal de cadastro para conter o botão com opção "cancelar" a operação

### Como reproduzir
Rode o projeto localmente e acesse a página /contas e clique no botão  "criar".
Após isso, a modal será exibida com o novo botão "cancelar".

Clique no novo botão para evitar a operação de cadastro.

### Resultado
Após clicar no botão o cadastro não deve ser realizado e a modal deve ser fechada.
Você pode verificar que a requisição de cadastro não ocorrerá após clicar nesse botão pela aba "network" de DevTools do seu navegador.

<screenshot-da-tela-com-a-alteração>

No dia-a-dia isso depende muito do time e do contexto que você trabalha, pode ser que seu projeto siga um padrão de templates para descrição ou título pra qualquer mudança que venha a ser proposta e isso já ajuda bastante.

Caso não siga um padrão, talvez seja uma boa oportunidade pra adotar um. Independente do que acontecer, pode ser interessante detalhar seu PR com um pouco mais de profundidade, especialmente se estiver contribuindo pra algum projeto open-source.

Abra espaço para discussões trazendo proposições e não imposições

Muito dificilmente você vai querer abrir um PR com imensos ajustes sem nem ao mesmo entender como resolvê-los direito, ainda mais se estiver em um projeto novo.

Tente deixar o ambiente de revisão mais leve e abrir espaço para discussões. Revisão de código também serve pra debate técnico e você pode se surpreender com os ensinamentos que podem sair de algumas threads por aí.

Vamos ver a diferença de um pedido de alteração mais impositivo:

Ajustar declaração da variável pra usar destructuring.

Olha como a leitura muda completamente, só de fazer uma leve adaptação no texto, tornando-o muito mais propositivo:

O que acha de ajustar a declaração da variável pra usar destructuring?

Apenas de adaptar o pedido de alteração pra uma pergunta, você já consegue abrir um espaço pra algumas discussões positivas. Isso dá abertura pra pessoa que criou o PR ter como se expressar, afinal ela também passou um tempo desenvolvendo o código que você está revisando.

Embora seja uma alteração pequena, a forma como a outra pessoa vai ler essa mudança é muito diferente. Da segunda maneira, seu ajuste deixa de ser uma imposição de uma pessoa que possui certo contexto e influência sobre um projeto e passa a ser uma proposta de solução, deixando a situação muito mais amigável.

Detalhe "por quê" e o "como", não somente o "que" deve ser feito

Talvez seja o tópico que possa "tomar mais tempo" enquanto estamos realizando uma revisão, mas acho que ainda assim é fundamental, ainda mais quando existem membros novos na equipe.

Detalhar o motivo e como uma alteração deve ser feita no código, na minha opinião, é tão importante quanto a própria alteração em si.

Exemplos deixam explicações muito mais claras

Essa abordagem pode ficar ainda mais clara se você trouxer alguns exemplos.

Já ocorreu uma discussão parecida recentemente? Gostaria de usar algum post como base pra alteração que está propondo? Esse padrão já foi adotado em outra parte do projeto?

Por que não compartilhar todo esse material?

Com isso, você traz pra pessoa que abriu o PR todo o contexto que antes estava claro somente para você.

Isso tem um impacto ainda mais positivo se a pessoa for nova no time, fazendo com que ela fique por dentro das discussões e histórico de decisões.

Vamos ver como aplicar isso no comentário abaixo:

O que acha de ajustar a declaração da variável pra usar destructuring?

Só alterar:
<code>
const modulo = require('modulo');
const funcao = outroModulo.funcao;
</code>

Para:
<code>
const modulo = require('modulo');
const { funcao } = outroModulo.funcao;
</code>

[Recentemente](link-da-thread-do-slack) adotamos isso como um padrão dentro do projeto.
Os [arquivos mais novos](link-de-outro-arquivo-com-o-padrao) já seguem isso.

O comentário fica muito mais rico em explicações e compartilha um contexto que pode não ser claro pra quem fez a alteração do código. Algumas plataformas mesmo permitem que você sugira uma alteração e a própria pessoa que escreveu o PR possa realizar o commit dessas alterações direto pela interface do pull-request.

Não esqueça de retornar às discussões que você abriu

Esse ponto eu falo por experiência própria, já foi ponto negativo em feedbacks que tive anteriormente.

É normal nos perdemos em algumas discussões especialmente se elas são longas, né?

Por mais que seja cansativo, ninguém quer ser a pessoa que fica esperando eternamente uma resposta numa discussão simplesmente por ser uma situação extremamente chata.

Quando você espera a resposta de alguém, as coisas ficam mais delicadas. Fica difícil de saber se você deve pressionar aquela pessoa via Slack (ou qualquer ferramenta de comunicação da sua empresa) pra revisitar o PR ou se algo está acontecendo.

Acostume-se a começar e encerrar as discussões que você participa em uma revisão. Se acha que a discussão já terminou com ou sem algum ajuste necessário, algumas plataformas permitem que você encerre um comentário clicando

Se a sua discussão foi levada para alguma reunião e decidida com o time, você também pode voltar na thread e comentar algo como:

Discussão resolvida via [slack](link-se-tiver).

Com isso, você também pode manter esse histórico atualizado.

Não faz mal contribuir de forma ativa

Explicar algo pessoalmente sempre vai ser muito mais prático e certeiro do que achar as palavras certas na hora de escrever.

Muitas vezes uma chamada de 5 minutos resolve horas de discussão escrita. Se achar que vai ser mais produtivo fazer um pair-programming ou realizar o ajuste você mesmo, pode ser uma ótima solução!

Dessa forma você pode aprender mais sobre a forma como a pessoa que abriu o PR raciocinou para resolver o problema e também validar pontos positivos e negativos de forma conjunta, independente de quem for realizar o commit no fim das contas.

Lembre-se da qualidade de código

Além de se certificar que uma funcionalidade foi entregue como deveria, qualidade é um ponto que sempre demanda uma atenção extra.

Lembre-se de olhar com carinho se as regras de formatação e padronização estão sendo seguidas corretamente, se os testes foram escritos e estão cobrindo pelo menos os cenários e os arquivos mais importantes daquela funcionalidade.

Não é novidade que em muitas empresas as pessoas que revisam código também possuem certa responsabilidade por aquela implementação que foi adicionada. Ninguém vai querer acordar por causa de algum erro bobo de implementação que poderia ter sido descoberto facilmente por um teste unitário, então manter um olho atento aos padrões de qualidade do projeto também é muito importante.

Você também pode comentar no seu próprio PR

Achou algum trecho de código um pouco complicado de entender? Não conseguiu achar uma alternativa melhor pra alguma solução? A própria documentação da ferramenta que está sendo usada está confusa?

Além de comentar blocos e trechos diretamente no código, você também é livre pra comentar seu próprio pull-request.

Não tenha vergonha de pedir alterações pra você mesmo ou revisar e escrever algum comentário no seu código (talvez até abrir um PR em rascunho?), caso alguma implementação precise ser alterada. Isso é ainda mais importante se seu time tiver costume de fazer merge de PRs de outras pessoas, dessa forma você consegue sinalizar que seu trabalho ainda precisa de uma atenção.

Torne essa dinâmica em algo mais positivo

Provavelmente boa parte do seu tempo, ao trabalhar com desenvolvimento, vai ser revisando código. Por que não tentar deixar essa tarefa mais leve?

Espero que alguns dos pontos acima possam ter sido um pouco interessantes e, quem sabe, te ajudem a repensar suas revisões, independente de qual lado do pull-request você estiver!


Compartilhe