Revisar Pull Requests é sempre um desafio: como manter a qualidade e uniformidade do código, mas ao mesmo tempo manter motivada a pessoa que desenvolveu? Será que colocar etiquetas ou categorias nas suas mensagens facilita a interpretação?
Recentemente tuitei sobre o site https://conventionalcomments.org/. A ideia é simples: toda vez que estiver fazendo a revisão de um PR, suas mensagens devem começar com uma etiqueta, para deixar claro qual é o próximo passo esperado.
O que é um Pull Request (ou Merge Request)?
Pull Requests são solicitações para que algum código faça parte do código “oficial” do projeto. Dependendo da ferramenta, essas solicitações também podem ser chamadas de Merge Requests.
Acontece assim:
- A Pessoa A:
- Escreve código para uma determinada alteração e
- Abre um PR, descrevendo o que e por que está sendo mudado;
- Um processo de revisão e adequação acontece. Parte desse processo pode depender da revisão de outra Pessoa B.
- O código então é incorporado.
Este post é sobre as mensagens da Pessoa B para a Pessoa A explicando quais adequações precisam acontecer antes que o código seja incorporado ou, como falamos no dia a dia, que se possa dar merge no código.
AVISO: Existe muito material sobre boas práticas na hora de revisar um PR. Esse post não cobre esse assunto, mas recomendo demais procurar material sobre isso também. Embora não seja exatamente sobre isso, recomendo esse vídeo do Dave Farley sobre liderança de equipes e como é importante permitir que as pessoas resolvam problemas de formas diferentes de você.
Prefixos e etiquetas: Acordo sobre o formato
O site sugere o seguinte formato para os comentários da revisão do Pull Request:
<etiqueta> [decoradores]: <assunto> [discussão]
Por exemplo:
suggestion (test,if-minor): Parece que a cobertura de testes não verifica se o gato pode sumir completamente.
Será que a gente consegue escrever esse teste antes de dar merge no PR?
Dando uma olhada no exemplo, temos:
Label (Ex.: suggestion):
A etiqueta indica que o comentário está relacionado a uma sugestão, algo que o revisor acredita que pode ser melhorado. O que e por que deve ser comunicado.
Decoradores (Ex.: test, if-minor):
Um jeito rápido de expandir o significado da etiqueta. O comentário é sobre testes e a decisão sobre bloquear ou não o merge cabe ao autor, dependendo do tamanho do problema.
Assunto:
A primeira linha, onde a primeira explicação é fornecida.
Discussão:
Não é um item obrigatório e, no meu exemplo, é supérfluo. Serve só para expandir o assunto e explicar melhor o que se precisa.
O site tem uma lista bem completa de sugestões para labels e decorators.
Dois destaques: nitpick e praise
Da lista sugerida existem dois labels que eu achei bem interessantes: nitpick e praise.
O label nitpick serve para quando você só quer chamar a atenção para alguma coisa que você faria diferente, mas que não impede de dar merge no código da forma que está. Nitpick é a expressão em inglês usada quando alguém só está sendo chato com algo sem importância. É o clássico “eu faria diferente, mas tudo bem”.
O label praise serve para elogiar. Esse é um dos jeitos de reconhecer (sem falsidade) alguma solução bacana, ou algum código criativo ou elegante. Como cada comentário gera uma “pendência” no PR do GitHub, por exemplo, a equipe pode convencionar que o autor pode “resolver” o comentário logo depois de ler.
E aí, o que você acha da ideia? Deixa aqui nos comentários ou nas minhas redes sociais!
