Code Review Compile

Conteúdo

  • O que é?
  • Como pode ser feito?
  • Exemplo de pull request
  • Responsabilidades do coder
  • Responsabilidades do reviewer
  • Sugestão de checklist
  • Boas práticas
  • Motivos para não fazer
  • Benchmarks

O que é?

Como pode ser feito?

Pull Request/Merge Request

GIT Lab

Responsabilidade do Coder

  • Deixar claro para o time o objetivo técnico ou de negócio do pull request aberto.
  • Realizar anotações no pull request antes mesmo do review iniciar, apontando os motivos por trás das decisões técnicas e um guia por onde iniciar a análise de código, papel dos módulos ou modificações diante o objetivo a ser alcançado.
  • Evitar submeter pull requests gigantescos.
  • Estar aberto a receber feedbacks construtivos de todos, independente do cargo e função.
  • Compartilhar conhecimento.

Responsabilidades do Reviewer

  • Pensar na qualidade e bem do produto acima de gostos pessoais.
  • Apontar especificamente os motivos e citar exemplos de uma melhor solução quando o caso.
  • Não ser agressivo ao apontar pontos de melhorias.
  • Ter em mente que cometer erros faz parte da evolução e não deve ser transformado em uma piada ou algo pejorativo.
  • Não realizar a implementação dos pontos de melhoria por mais simples que sejam - Aumento de Lead Time.

Alinhamento de Expectativas

Sugestão de CheckList

CheckList

Boas Práticas

  • Comece com um checklist simples - Evolução > Revolução
  • Tente submter um P.R com menos de 400 linhas de mudança.
  • Mensagens de commit claras e explicativas.
  • Deixe claro o propósito do seu código.
  • Faça sugestões de melhoria e não aponte apenas erros.
  • Responda e discuta cada comentário.
  • Qualidade e padrões do time > Gosto pessoal.
  • Muito tempo investido na revisão + No LWIP podem gerar gargalo no fluxo do time.
  • Critique positivamente o código, não o autor.
  • Tome como oportunidade para reconhecer com elogios o esforço e empenho do coder em entregar com qualidade.
  • Evite julgamento absolutos: “isso nunca vai funcionar”, “sempre dá erro”.

Motivos para não fazer

  • Eu não tenho tempo...
  • Eu não tenho conhecimento o bastante para revisar código...
  • Eu não preciso que revisem minhas coisas, meu código é execelente!
  • Não posso fazer esse review, eu não conheço nada sobre o módulo XPTY....

Four NOs of a Serious Code Reviewer

No Fear

"I'd better close my eyes and pretend I didn't see her bugs today so tomorrow she will ignore my mistakes"

 

"I may be wrong and they will laugh me out" Even worse, they may spot my lack of knowledge"

 

"If I reject this code, I will delay the release"

 

 

Be direct, honest, and straight-forward. If you don't like the code, you don't like it.

 

They expect you to be smart and bright,  but you won't get there if you don't learn from your mistakes

Do the right thing and say what you really think.

No Compromise

"Let's make peace just to stop fighting"

I won't  accept this, because x, y and z.

 

You're right, i'm gonna make some changes.

 

Let's ask about the architect opinion.

No Bullshit

"I know this because I've been writing Java for 15 years"

In most cases, convincing is teaching:

 

You need to show proof. You need to ground your logic and make sure he understands and accepts it.

 

Be ready to show links, articles, books, reports, examples, etc

 

If you don't have enough convincing proof, think again—maybe you are wrong.

No Offense

"You should delete this garbage and re-write it from scratch"

No matter how bad the code in front of you is, be patient and convincing, be professional.

Benefícios

  • Identificar previamente problemas de código e pontos fracos de qualidade.
  • Nivelamento de conhecimento técnico e de negócio dentro do time.
  • Multidisciplinaridade.
  • Novos métodos de otimização ou técnicas se espalham mais rápidos dentro do time.

Slide Martin’s Fowler (talk presented at OOP 2014):

Benchmarks

  • IBM found that each hour of inspection prevented about 100 hours of related work (testing and defect correction) (Holland 1999).
  • Hewlett-Packard reported that its inspection program saved an estimated $21.5 million per year (Grady and Van Slack 1994).
  • Imperial Chemical Industries found that the cost of maintaining a portfolio of about 400 programs was only about 10 percent as high as the cost of maintaining a similar set of programs that had not been inspected (Gilb and Graham 1993).
  • A study of large programs found that each hour spent on inspections avoided an average of 33 hours of maintenance work and that inspections were up to 20 times more efficient than testing (Russell 1991).
  • A group of 11 programs were developed by the same group of people, and all were released to production. The first five were developed without reviews and averaged 4.5 errors per 100 lines of code. The other six were inspected and averaged only 0.82 errors per 100 lines of code. Reviews cut the errors by over 80 percent (Freedman and Weinberg 1990).

Referências

  1. http://blog.8thcolor.com/en/2014/04/5-reasons-you-are-not-doing-code-reviews/
  2. https://www.yegor256.com/2015/02/09/serious-code-reviewer.html
  3. https://www.yegor256.com/2015/01/26/happy-boss-false-objective.html
  4. https://smartbear.com/learn/code-review/best-practices-for-peer-code-review/
  5. http://amanek.com/why-your-team-should-do-a-peer-review-on-a-regular-basis/
  6. https://tech.olx.com/may-the-code-review-be-with-you-3407955e4c19

Code Review

By Raphael Carvalho

Made with Slides.com

Code Review

  • 2,091

More from Raphael Carvalho