Code REview

Revisão

avaliação formal de algo com a intenção de promover mudanças, se necessário.

Code Review

Análise do código fonte com intuito de achar defeitos:

Segurança, Funcional, Performance...
Complementa a análise estática.
Detecção antecipada.

COMPLIANCE

PCI-DSS - Payment Card Industry Data Security Standard (since 2005 - version 3.0)
Requirement :
“Review custom code prior to release to producion or customers in order to identify any potencial coding vulnerability (using either manual or automated processes) …“

Metodos

Os métodos de code review podem variar muito de acordo com a profundidade da análise.

Formal

Linha a linha.
Múltiplos reviewers.
Review em grupo.
Cópias impressas.
Encontra problemas difíceis de encontrar.
Gasta tempo.

Leve

Análise superficial.
Análise baseadas em padrões.
Revisa apenas funções críticas.
Fácil de deixar passar problemas mais simples.
Gasta menos tempo.
Bom para encontrar vunerabilidades básicas.

Como pode ser feito

Manual
Automática
Ambos

3 Regras para um REview

Regra 1

O reviewer não pode ser quem escreve o código.

Se somos capazes de achar bugs no código, somos capaz de evitá-los.
Análise Parcial.
O reviewer deve ter uma perspectiva diferente do desenvolvedor.
O reviewer (preferencialmente) deve participar de outro projeto.

Regra 2

O reviewer deve entender a linguagem sendo revisada.

Regra 3

Foco no Objetivo: segurança, performance, funcionalidade, etc.

Não deve basear-se em preferências pessoais.

Regra 4?

Não procure apenas por defeitos, elogie quando achar coisas legais e tente aprender com essa oportunidade.

Um pouco mais sobre motivaçÃo

como motivar os reviewers?

Dizer

"Code review é obrigatorio" não vai funcionar!

Os devs tem coisas mais interessantes para fazer.
Os devs tem mais o que fazer.
Os devs tem deadlines agressivos e a revisão acaba facilmente sendo desconsiderada.
Os devs não gostam do código alheio.
Os devs não sabem O QUE revisar.

A grande Pergunta

O que revisar?

Não deixem os devs escolherem o que revisar arbitrariamente.

Code review são para Reviewers

Utilize uma ferramenta para gerenciar o que é associado para cada reviewer.
Cada reviewer tem uma fila de revisões a serem feitas.
1 revisão = 1 Pull Request

garanta

Cobertura de código - Todo o código deve ser revisado.
Responsabilidade - O desenvolvedor tem uma tarefa publicamente associada a ele.
Entregáveis - Crie evidências, aumente a motivação para revisar.

Fluxo de Trabalho

BRANCH - DISCUSS - MERGE

BRANCH

DISCUSS

MERGE

PRECISAMOS FALAR SOBRE DESIGN REVIEW

Design Review

É muito frustrante descobrir ao fim de uma implementação que o design é ruim e ter que troca-lo.

Até mesmo deixar o design ruim e não escalavel, com isso nossa code-base piora inegavelmente, pois o custo de manutenção aumenta exponencialmente com o tempo.

Como fazer?

Durante a fase de analise da issue, utiliza-se de um um outro dev para discutir a idéia do que será implementado.

(Dev que auxilia pode (deve?) lançar horas na issue do colega em casos de analises mais complexas)

Exceções

Post-Commmit Review

A atividade de code review tardio.
O que não deveria acontecer, mas vem se tornando comum, ou pela falta de capacitados para a tarefa de code review ou pela urgência de algumas entregas.

Faz-se a entrega e cria-se uma tarefa de débito técnico na ferramenta de Issue Tracker para a revisão do código entregue sem revisão.

(Alinhe essa necessidade com sua equipe/time)

Eu ouvi ?