Como revisar códigos sem ofender outros desenvolvedores [duplicados]

37

Eu trabalho em uma equipe que faz revisões freqüentes de código. Mas parece mais uma formalidade do que qualquer coisa.

Ninguém realmente aponta problemas no código por medo de ofender outros desenvolvedores. As poucas vezes em que tentei pedir mudanças foram recebidas com atitudes muito defensivas e relutantes.

Isso obviamente não é bom. Não só estamos gastando tempo para rever o código, mas estamos obtendo literalmente zero valor dele. Esta é uma questão que precisa ser tratada por desenvolvedores individuais ou existem técnicas para sugerir mudanças sem pisar nos dedos dos outros?

    
por ConditionRacer 07.10.2013 / 16:26
fonte

5 respostas

61

Esta parece ser uma atitude predominante entre alguns desenvolvedores. Todos parecem achar que uma revisão de código é um desafio para o trabalho deles, e isso não faz sentido para mim. Uma revisão de código é um mecanismo de garantia de qualidade que tem a vantagem adicional da educação para acompanhá-lo. Implementamos revisões de código extensivamente onde eu trabalho, e incentivei dentro de minha própria equipe a atitude de que as revisões de código são um mecanismo de colaboração mais do que um processo de qualidade.

A única maneira de começar a codificar como equipe é ver o trabalho do outro e questioná-lo. É assim que as melhores práticas são formadas. Diálogo é a chave. Enviei o código de volta aos desenvolvedores por motivos bobos, como formatação, melhores práticas e ortografia. Eu codifico a revisão com uma borda muito fina e espero que meu próprio código resista ao mesmo escrutínio. Eu usei táticas como o check-in de código que não passaria na minha própria revisão de código com o único propósito de fazer com que um desenvolvedor iniciante me desafiasse . Diga-me que não atende aos requisitos. Levante-se e tenha uma opinião.

Algumas diretrizes que todos devem ter ao participar de um ambiente de revisão de código:

  1. Enfrente você mesmo. Você não é perfeito, e só porque um desenvolvedor júnior conseguiu vê-lo fazendo algo bobo em um loop não é o fim do mundo. Se você é tão bom, peça críticas e prove. Espere revisões de código para revelar novas idéias sobre como fazer coisas, novos hábitos e, acima de tudo, diálogo construtivo.
  2. Mantenha a mente aberta ao revisar o código de outra pessoa. Seja receptivo às suas ideias e certifique-se de que as alterações sugeridas sejam sugeridas por um motivo. Assume-se que o código com check-in é construído, mas isso não significa que ele segue as práticas recomendadas. Certifique-se de que qualquer coisa que você abrir possa ter um backup com uma referência citada. Se você disser que não segue as práticas recomendadas, cite o documento e a seção de padrões. Se você disser que não é um método de "desempenho", tenha um link para um documento que mostre por que e possivelmente forneça métricas.
  3. Faça sugestões úteis e explique por que você está sugerindo algo. Ocasionalmente, você encontrará um problema com código que é auto-explicativo, mas na maioria das vezes essa pessoa codificou algo baseado em hábitos. Explique por que esse hábito deve ser alterado e o valor de alterá-lo (a menos que você o explique pela 5ª vez, no qual você tem um problema pessoal).
  4. Quando seu código é revisado, considere tudo o que o revisor está sugerindo objetivamente. Se você está empurrando para trás, pergunte a si mesmo honestamente se você está apenas sendo defensivo ou se realmente acredita que tem um caso. Se você tiver um caso, continue com o diálogo. Não seja argumentativo, traga munições como fatos e métricas.
  5. Seja você um revisor ou um residente, use as revisões de código como uma oportunidade para educar. Seja educando a si mesmo ou a pessoa que você está revisando, se houver uma discrepância, existe a chance de aprender em algum lugar. Faça bom uso disso.
  6. Faça perguntas e esteja pronto para que suas perguntas sejam respondidas com sinceridade. Recentemente, fiz uma declaração que era "pseudo-verdadeira". Não estava errado, mas definitivamente não estava certo. Um desenvolvedor júnior me desafiou e eu discordei. Minha resposta foi "Eu não vi esse comportamento, mas se você puder me encontrar um documento sobre isso eu adoraria lê - lo". Eu passei cerca de uma hora naquela tarde lendo o documento que ele me enviou, e agora eu tenho uma resposta muito melhor (re: educado) quando confrontado com uma situação semelhante.

Dada a inclinação da educação na qual eu coloco as revisões de código, muitas vezes faço minhas respostas sobre perguntas. Se eu encontrar algo descaradamente, vou instruir o desenvolvedor a corrigi-lo. Caso contrário, farei as perguntas. "Por que você usou o método A para atingir o objetivo B?" "Qual ganho declarar uma variável no caso de seu uso no método Z?" "Eu vejo que você copiou / colou algum código, você considerou a refatoração? Você não refatorou, qual foi o raciocínio por trás dessa escolha?" O código não progride até que o revisor o aprove, e o revisor não o aprova até que as perguntas sejam respondidas. Quando enquadrado de uma forma inquisitiva que indica que você não entende o raciocínio dos desenvolvedores, ele se torna menos conflituoso e assume mais uma vibração instrucional.

    
por 07.10.2013 / 17:35
fonte
10

Para ajudar as pessoas a fazer uma revisão de código menos pessoal, tente direcionar críticas e sugestões para o código em si e não para o autor. Por exemplo,

The function "getFoo" should do xyz because....

é melhor que

You should do xyz in function "getFoo" because...

Por outro lado, se você quiser expressar uma opinião positiva, pode elogiar o autor. Por exemplo,

I like how you used abc algorithm because ...

    
por 07.10.2013 / 19:11
fonte
3

Primeiro, a equipe deve concordar com o objetivo da revisão do código (você não deve considerar esse contrato como garantido, pois a revisão de código pode ser usada para uma variedade de coisas, e organizações diferentes o fazem por diferentes razões. , Vou assumir que o objetivo é melhorar o trabalho futuro de um desenvolvedor.

Para melhorar, o desenvolvedor precisa saber

  1. o que foi bom? - > Eu deveria continuar fazendo isso
  2. o que foi ruim? - > Eu deveria mudar isso
  3. e por quê? - > Assim, posso avaliar a gravidade do problema e reconhecer problemas semelhantes no futuro

Sobre o que é essa ação?

O feedback deve ser sobre um comportamento, não uma pessoa. Portanto, deve identificar (preferencialmente com exemplos) o comportamento do feedback. Então não diga:

You're lazy.

porque não se trata de comportamento, mas de personalidade (percebida). Além disso, é vago demais: é sobre o longo intervalo para o almoço? Digitação lenta? Não respondendo e-mail em tempo hábil? Muito poucos recursos implementados? Em vez disso, você deve dizer:

Last week, you were assigned bug #1059. You have marked this defect resolved, but the bug still exists, and can be reproduced with the instructions provided in the trouble ticket.

E por quê?

Em seguida, você deve descrever por que essa ação ou código foi bom ou ruim. A razão dada deve ser objetiva e de importância suficiente para justificar uma mudança. Por exemplo:

AClass.amethod() is invoked by concurrent threads, but not thread safe. Data corruption is likely.

ou

This method name is misleading. It sounds as if we're merely calulating the pay grade, but actually also pays the employee. Somebody not familiar with the implementation is likely to misuse the method, causing accidental salary payments ...

É claro que às vezes você não tem um argumento rígido porque o código é ruim (ou bom). Isso significa que você não sabe que está certo. Portanto, é prudente ser curioso, em vez de presunçoso:

This looks awfully complex. Why don't you just invoke AClass.amethod() instead?

ou

This appears to be nearly identical to AClass.amethod(). Why can we not reuse that method?

Você também pode dar um feedback positivo, é claro:

That's a great approach to coordinate the worker threads! I usually do X instead, and I always wondered how I could make it more robust. Now I know :-)

Por favor, não faça isso

Para ilustrar a importância do acima, vamos considerar um contra-exemplo do mundo real feito textualmente a partir de uma revisão de código formal:

The code leaves a poorly thought out impression.

E foi tudo o que o revisor escreveu sobre essa descoberta.

Esse é um feedback terrível em muitos níveis:

  1. O feedback implica algo desfavorável sobre o autor (ou seja, que ele pensa mal)
  2. O feedback é sobre uma impressão. Isso não é objetivo.
  3. O feedback não identifica nenhuma parte do código que precisa ser alterada
  4. nem dê qualquer motivo para que isso aconteça.

O destinatário de tal "feedback" não pode saber o que ele precisa mudar, nem por que, e assim não aprender nada. Na verdade, ele não pode nem mesmo saber que o feedback é justo e, dada a conclusão infundada e infundada, é muito mais provável ver isso como um ataque pessoal de alguém que quer se elevar empurrando seus colegas para baixo. É provável que haja gritos.

    
por 07.10.2013 / 22:54
fonte
0

Uma revisão de código não deve ser apenas uma falha / aprovação, com apenas falha ao retornar um feedback detalhado. Isso tende a se concentrar no negativo e evita que as pessoas se exponham às críticas.

Os revisores também podem dar uma resposta mais negativa para conseguir mais atenção aos olhos do gerente em termos de "olha, estou fazendo meu trabalho e criticando tudo o que se depara com minha mesa"

Se você adicionar um campo de comentário retornado ao responsável pelo envio falhar ou passar, ele se sentirá mais positivo sobre o trabalho dele e o revisor ainda poderá fornecer feedback sobre o que ainda pode ser melhor, mesmo que o código passe na revisão. Isso também permitiria ao revisor fazer um refatorador limitado do código (formatando, renomeando variáveis) sem precisar incomodar o desenvolvedor pelas mudanças triviais além de notificá-lo sobre elas.

    
por 08.10.2013 / 10:01
fonte
0

Eles dizem que "o código deve pertencer à equipe", portanto, uma revisão não é tanto um ataque pessoal a um codificador individual, mas uma etapa de qualidade para garantir que as coisas sejam entendidas pelo restante da equipe. Eu diria que a maioria dos lugares que trabalhei é compreendida e aceita.

No entanto, eu entrevistei um cara que era tão contra a ideia de rever o código de outras pessoas que o rejeitamos como candidato! (isto é - não de ter seu código revisado, mas fazendo revisões ... foi estranho).

Isso me fez pensar um pouco sobre o problema de como algumas pessoas abordam as avaliações. Eu diria que uma maneira melhor de fazer revisões de código é lembrar a primeira linha da minha resposta e aproveitar isso. Para fazer isso é bem simples: uma revisão de código deixa de ser a percepção de que "alguém verifica meu código e me diz o que fiz de errado" e se torna uma oportunidade para o codificador dizer a outra pessoa o que o código faz, como e quais mudanças eles fizeram.

Nesse caso, o codificador terminará revisando seu próprio código, com um par extra de olhos para ajudar a identificar o bit que pode perder. É também uma chance para o programador explicar por que ele fez as coisas de uma certa maneira. Tal abordagem é tão eficaz (se não mais como você tem uma boa troca de comunicação) e consideravelmente mais amigável. Eu não acho que levaria mais tempo considerando que a revisão deveria ser mais rápida, pois o codificador original pode explicar as mudanças diretamente. As pessoas também devem entender a necessidade de tal revisão, pois estão "entregando" o conhecimento a outro membro da equipe que pode ser obrigado a manter o código no futuro.

Você pode aprimorar isso fazendo com que o 'revisor' venha preparado com os requisitos originais da alteração e marque-os conforme a solução for detalhada, para que se sinta ainda menos como uma revisão de código.

    
por 08.10.2013 / 16:57
fonte