Como manejo a discordância em uma revisão de código em relação a um caso de borda improvável?

186

Estou trabalhando em uma startup de robótica em uma equipe de cobertura de caminho e depois de enviar uma solicitação de recebimento, meu código é revisado.

Meu companheiro de equipe, que está na equipe há mais de um ano, fez alguns comentários ao meu código que sugerem que eu faço muito mais trabalho do que acredito ser necessário. Não, eu não sou um desenvolvedor preguiçoso. Eu amo o código elegante que tem bons comentários, nomes de variáveis, recuo e manipula casos corretamente. No entanto, ele tem um tipo diferente de organização em mente que eu não concordo.

Vou fornecer um exemplo:

Eu passei um dia escrevendo casos de teste para uma mudança em um algoritmo de localização de transição que fiz. Ele sugeriu que eu lide com um caso obscuro que é extremamente improvável que aconteça - na verdade, não tenho certeza se é mesmo possível que isso ocorra. O código que eu fiz já funciona em todos os nossos casos de teste originais e alguns novos que eu encontrei. O código que eu fiz já passa pelas nossas mais de 300 simulações que são executadas todas as noites. No entanto, para lidar com este caso obscuro levaria 13 horas que poderiam ser melhor gasto tentando melhorar o desempenho do robô. Para ser claro, o algoritmo anterior que estávamos usando até agora também não lidou com esse caso obscuro e nem uma vez, nos relatórios de 40k que foram gerados, ele já ocorreu. Somos uma startup e precisamos desenvolver o produto.

Eu nunca tive uma revisão de código antes e não tenho certeza se estou sendo muito argumentativo; Eu deveria ficar quieto e fazer o que ele diz? Eu decidi manter a cabeça baixa e apenas fazer a mudança, mesmo que eu discorde totalmente que foi um bom uso do tempo.

Eu respeito meu colega de trabalho e o reconheço como um programador inteligente. Eu simplesmente discordo dele em um ponto e não sei como lidar com o desacordo em uma revisão de código.

Sinto que a resposta que escolhi atende a esse critério de explicar como um desenvolvedor júnior pode lidar com discordância em uma revisão de código.

    
por Klik 05.02.2017 / 01:46
fonte

22 respostas

225

an obscure case that is extremely unlikely to happen--in fact I'm not sure it is even possible to occur

Não ter comportamentos não testados no código pode ser muito importante. Se um pedaço de código é executado, por ex. 50 vezes por segundo, uma chance em um milhão acontecerá aproximadamente a cada 5,5 horas de tempo de execução. (No seu caso, as chances parecem menores.)

Você pode falar sobre as prioridades com seu gerente (ou com qualquer pessoa mais experiente responsável pela unidade em que você trabalha). Você entenderá melhor se, por exemplo, Trabalhar no desempenho do código ou código à prova de balas é a principal prioridade, e quão improvável esse caso pode ser. Seu revisor também pode ter uma ideia distorcida de prioridades. Tendo conversado com a pessoa responsável, você terá um tempo mais fácil (des) concordando com as sugestões do seu revisor, e terá algo para se referir.

É sempre uma boa ideia ter mais de um revisor. Se o seu código for revisado apenas por um colega, peça a alguém que conheça esse código, ou a base de código em geral, para dar uma olhada. Uma segunda opinião, novamente, ajudará você a mais facilmente (des) concordar com as sugestões do revisor.

Ter vários comentários recorrentes durante várias revisões de código geralmente indica que uma coisa maior não está sendo comunicada com clareza e os mesmos problemas aparecem repetidas vezes. Tente descobrir essa coisa maior e discuta-a diretamente com o revisor. Faça perguntas por que . Isso me ajudou muito quando comecei a prática de revisões de código.

    
por 05.02.2017 / 02:16
fonte
321

Eu posso dar um exemplo de um caso que nunca poderia ocorrer e que causou um desastre.

Quando o Ariane 4 estava sendo desenvolvido, os valores da lateral accelerometers foram dimensionados para caber em um número inteiro assinado de 16 bits e porque a saída máxima possível dos acelerômetros, quando escalonados, poderia nunca exceder exceder 32767 e o mínimo poderia nunca cair abaixo de -32768 houve "não há necessidade de a sobrecarga de verificação de intervalo". Em geral, todas as entradas devem ser verificadas antes de qualquer conversão, mas, neste caso, isso estaria tentando capturar um caso impossível.

Vários anos depois, o Ariane 5 estava sendo desenvolvido e o código para escalonar os acelerômetros laterais foi reutilizado com testes mínimos foi "comprovado em uso". Infelizmente, o foguete maior poderia esperar acelerações laterais maiores para que os acelerômetros fossem atualizados e pudessem produzir maiores 64 bits valores de float .

Esses valores maiores "empacotados" no código de conversão, lembram nenhuma verificação de intervalo, e os resultados no primeiro lançamento em 1996 não foram bons. Custou milhões à empresa e causou um grande hiato no programa.

Opontoqueestoutentandofazeréquevocênãodeveignorarcasosdetestecomonuncaacontecendo,extremamenteimprovável,etc.:ospadrõesparaosquaiselesestavamcodificandochamadoparaverificaçãodeintervalodeallvaloresdeentradaexternos.Seissotivessesidotestadoemanipulado,odesastrepoderiatersidoevitado.

NotaquenoAriane4issonãoeraumbug,(comotudofuncionoubemparatodososvalorespossíveis)-foiumafalhaemseguirospadrões.Quandoocódigofoireutilizadoemumcenáriodiferente,elefalhoucatastroficamente,enquanto,seointervalodevalorestivessesidocortado,provavelmenteteriafalhadonormalmenteeaexistênciadeumcasodetesteparaessepoderiadesencadearamumarevisãodosvalores.Éimportantenotartambémque,enquantooscodificadoresetestadoresvieramparaalgumascríticasdosinvestigadoresapósaexplosão,agestão,QA&liderançaficoucomamaioriadaculpa.

Esclarecimento

Emboranemtodosoftwaresejadesegurançacrítica,nemtãoespetacularquandoelefalha,minhaintençãoeradestacarquetestes"impossíveis" ainda podem ter valor. Este é o caso mais dramático que conheço, mas a robótica também pode produzir alguns resultados desastrosos.

Pessoalmente, eu diria que uma vez que alguém tenha destacado um caso de canto para a equipe de teste, um teste deve ser posto em prática para verificar isso. O líder da equipe de implementação ou o gerente de projeto pode decidir não tentar resolver as falhas encontradas, mas deve estar ciente de que existem falhas. Alternativamente, se o teste é muito complexo, ou caro, para implementar, um problema pode ser levantado em qualquer rastreador que estiver em uso & ou no registro de risco para deixar claro que este é um caso não testado - que pode ser necessário abordadas antes de uma mudança de uso ou impedir um uso inadequado.

    
por 05.02.2017 / 12:15
fonte
84

Como não foi tratado antes, está fora do escopo do seu esforço. Você ou seu colega podem perguntar ao seu gerente se vale a pena o esforço para cobrir este caso.

    
por 05.02.2017 / 02:51
fonte
53

Com algoritmos complexos, é muito difícil provar que você pensou em todos os casos de teste que aparecerão no mundo real. Quando você deixa intencionalmente um caso de teste quebrado porque ele não aparece no mundo real, você está potencialmente deixando outros casos de teste quebrados que você nem imaginou ainda.

O outro efeito que acontece com frequência é quando você lida com casos de teste adicionais, seu algoritmo necessariamente se torna mais geral e, por causa disso, você encontra maneiras de simplificá-lo e torná-lo mais robusto para todo seu teste casos. Isso economiza seu tempo em manutenção e solução de problemas no futuro.

Além disso, há muitos casos de "isso nunca deve acontecer" erros acontecendo na natureza. Isso porque seu algoritmo pode não mudar, mas suas entradas podem mudar anos a fio quando ninguém se lembra desse caso de uso não manipulado. É por isso que os desenvolvedores experientes lidam com esse tipo de coisa quando estão atualizados. Ele volta para te morder mais tarde, se você não fizer isso.

    
por 05.02.2017 / 03:21
fonte
38

Esta não é uma questão técnica, mas uma decisão de estratégia de negócios. Você percebe que a correção sugerida é para um caso muito obscuro, o qual quase nunca acontece. Aqui, faz uma grande diferença se você estiver programando um brinquedo ou se estiver programando um equipamento médico ou um drone armado. As consequências de um mau funcionamento raro serão muito diferentes.

Ao fazer revisões de código, você deve aplicar um entendimento básico das prioridades de negócios ao decidir quanto investir no tratamento de casos raros. Se você não concordar com seu colega em sua interpretação das prioridades do negócio, convém discutir com alguém do lado comercial.

    
por 05.02.2017 / 11:12
fonte
21

Revisões de código não são puramente sobre correção de código. Na realidade, isso está bem abaixo na lista de benefícios, por trás do compartilhamento de conhecimento e de ser um processo lento, mas constante, para criar um consenso de estilo / design da equipe.

Como você está experimentando, o que conta como "correto" é muitas vezes discutível, e todo mundo tem seu próprio ângulo sobre o que é isso. Na minha experiência, tempo e atenção limitados também podem tornar as revisões de código altamente inconsistentes - o mesmo problema pode ser detectado ou não dependendo de diferentes desenvolvedores e em momentos diferentes pelo mesmo desenvolvedor. Revisores na mentalidade de "o que melhoraria este código?" frequentemente sugerirão mudanças que eles não acrescentariam aos seus próprios esforços naturalmente.

Como codificador experiente (mais de 15 anos como desenvolvedor), muitas vezes sou revisado por codificadores com menos anos de experiência do que eu. Às vezes, eles me perguntam por mudanças que eu discordo levemente, ou acho que não é importante. No entanto, ainda faço essas alterações. Luto batalhas por mudanças apenas quando sinto que o resultado final causaria uma fraqueza no produto, em que o custo do tempo é muito alto, ou onde um ponto de vista "correto" pode ser feito de forma objetiva (por exemplo, a mudança sendo solicitada) t trabalhar na linguagem que estamos usando, ou um benchmark mostra que uma melhoria de desempenho alegada não é uma).

Então sugiro escolher suas batalhas com cuidado. Dois dias codificando um caso de teste que você acha que não é necessário provavelmente não vale o tempo / esforço para lutar. Se você estiver usando uma ferramenta de revisão, como solicitações de pull do GitHub, então talvez comente sobre custo / benefício que você perceba, a fim de anotar sua objeção, mas concorde em continuar a fazer o trabalho. Isso conta como um retrocesso suave, de modo que o revisor saiba que está atingindo um limite e, mais importante, inclua sua justificativa para que casos como esse possam ser escalados se entrarem em conflito. Você quer evitar a escalada de desacordos escritos - você não quer ter um argumento de estilo de fórum da Internet sobre diferenças de trabalho - então pode ser útil falar primeiro sobre o assunto e registrar um resumo justo do resultado da discussão, mesmo se você ainda concordam em discordar sobre a necessidade do trabalho (é inteiramente possível que uma breve discussão amigável decida por ambos).

Após o evento, este é um bom tópico para revisão de sprints ou reuniões de planejamento da equipe de desenvolvimento, etc. Apresente-o da forma mais neutra possível. "Na revisão de código, o Desenvolvedor A identificou este caso de teste adicional, levou dois dias extras para escrever, a equipe acha que a cobertura extra foi justificada a esse custo?" - esta abordagem funciona muito melhor se você realmente fizer o trabalho, como mostra uma luz positiva; você fez o trabalho e apenas deseja pesquisar a equipe em busca de aversão ao risco versus desenvolvimento de recursos.

    
por 05.02.2017 / 11:04
fonte
15

Eu aconselho você a, pelo menos, afirmar contra o caso obscuro. Dessa forma, os futuros desenvolvedores não apenas verão que você decidiu ativamente contra o caso, mas com um bom manuseio de falhas, que já deveria estar em vigor, isso também surpreenderia.

E, em seguida, faça um caso de teste que declare essa falha. Desta forma, o comportamento é melhor documentado e aparecerá em testes unitários.

Esta resposta, obviamente, assume que o seu julgamento do mesmo ser "extremamente improvável, se possível" está correto e não podemos julgar isso. Mas se for, e seu colega concordar, então uma afirmação explícita contra o evento deve ser uma solução satisfatória para ambos.

    
por 05.02.2017 / 15:09
fonte
13

Desde que você parece ser novo lá, há apenas uma coisa que você pode fazer - verifique com o líder da equipe (ou líder do projeto). 13 horas é uma decisão de negócios; para algumas empresas / equipes, muito; para alguns, nada. Não é sua decisão, ainda não.

Se o líder disser "cobrir esse caso", tudo bem; se ele diz "nah, estrague tudo", tudo bem - a decisão dele, a responsabilidade dele.

Quanto às revisões de código em geral, relaxe. Ter uma tarefa devolvida a você uma ou duas vezes é perfeitamente normal.

    
por 06.02.2017 / 02:16
fonte
7

Uma coisa que eu acho que não vi foi abordada em espécie, embora tenha sido criada na resposta do @SteveBarnes:

Quais são as repercussões de um fracasso?

Em alguns campos, uma falha é um erro em uma página da web. Um PC azul telas e reinicia.

Em outros campos, é a vida ou a morte - o carro que dirige sozinho trava. O pacemaker médico deixa de funcionar. Ou na resposta de Steve: as coisas explodem, resultando na perda de milhões de dólares.

Existe um mundo de diferença nesses extremos.

Se 13 horas para cobrir uma "falha" vale a pena, em última análise, não cabe a você. Deve ser da gerência e dos proprietários. Eles devem ter uma idéia do quadro maior.

Você deve ser capaz de dar uma boa ideia do que valerá a pena. Seu robô simplesmente diminuirá a velocidade ou parará? Desempenho degradado? Ou será que uma falha do robô causará danos monetários? Perda de vida?

A resposta a essa pergunta deve direcionar a resposta para "vale 13 horas do horário das empresas". Aviso: Eu disse o horário das empresas. Eles pagam as contas e, finalmente, decidem o que vale a pena. Sua gerência deve dar a palavra final de qualquer forma.

    
por 07.02.2017 / 03:57
fonte
5

Talvez conversar com a pessoa responsável por priorizar o trabalho? Na inicialização, pode ser o CTO ou o proprietário do produto. Ele poderia ajudar a descobrir se esse trabalho extra é necessário e por quê. Além disso, você pode trazer suas preocupações durante os levantamentos diários (se você os tiver).

Se não houver responsabilidade clara (por exemplo, proprietário do produto) para planejar o trabalho, tente conversar com as pessoas ao seu redor. Mais tarde, pode se tornar um problema que todos estão levando o produto para a direção oposta.

    
por 05.02.2017 / 11:55
fonte
5

A melhor maneira de lidar com a discordância é a mesma, independentemente de você ser um desenvolvedor júnior ou um desenvolvedor sênior, ou até mesmo um CEO.

Aja como o Columbo .

Se você nunca viu nenhum Columbo, foi um show fantástico. Columbo era esse personagem muito despretensioso - a maioria das pessoas pensava que ele era um pouco louco e não vale a pena se preocupar. Mas parecendo humilde, e apenas pedindo às pessoas que explicassem, ele foi capaz de conseguir o seu homem.

Acho que também está relacionado ao método socrático .

Em geral, você quer fazer perguntas a você mesmo e aos outros para se certificar de que está fazendo as escolhas certas. Não de uma posição de "estou certo, você está errado", mas de uma posição de descoberta honesta. Ou pelo menos da melhor maneira possível.

No seu caso, você tem duas ideias aqui, mas elas têm basicamente o mesmo objetivo: melhorar o código.

Você tem a impressão de que poupar na cobertura de código para um caso potencialmente improvável (impossível?) em favor do desenvolvimento de outros recursos é a melhor maneira de fazer isso.

Seu colega de trabalho tem a impressão de que ter mais cuidado com os casos de canto é mais valioso.

O que eles veem que você não vê? O que você vê que eles não veem? Como um desenvolvedor júnior, você está realmente em uma ótima posição, porque você naturalmente deve fazer perguntas. Em outra resposta, alguém menciona quão surpreendentemente provável é um caso específico. Então você pode começar com: "Ajude-me a entender - fiquei com a impressão de que X, Y e Z - o que está faltando? Por que o framboozle widget? Eu estava sob a impressão de que seria fintável em circunstâncias cromulent. Will o bastão swizzle realmente embiggen os pincéis ANZA? "

Ao questionar suas suposições e suas descobertas, você descobrirá quais são os vieses e, eventualmente, descobrirá qual é o curso de ação correto.

Comece com a suposição de que todos em sua equipe são perfeitamente racionais e eles têm os melhores interesses da equipe e do produto em mente, assim como você. Se eles estão fazendo algo que não faz sentido, então você precisa descobrir o que você não sabe que eles fazem, ou o que você sabe que eles não sabem.

    
por 07.02.2017 / 15:07
fonte
3

13 horas não é tão importante, eu apenas faria isso. Lembre-se que você está sendo pago por isso. Apenas giz-lo como "segurança no emprego". Também é melhor manter um bom carma entre a equipe. Agora, se fosse algo que levaria uma semana ou mais, você poderia envolver seu gerente e perguntar se esse é o melhor uso do seu tempo, especialmente se você não concordar com isso.

No entanto, parece que você precisa de influência no seu grupo. Veja como você consegue alavancagem: peça perdão, não peça permissão. Acrescente coisas ao programa como achar melhor (dentro do escopo do curso, ou seja, certifique-se de que resolve completamente o problema que o chefe deseja), e diga ao gerente ou seus colegas depois do fato. Não pergunte a eles: "Tudo bem se eu adicionar o recurso X". Em vez disso, basta adicionar recursos que você deseja pessoalmente no programa. Se eles ficarem chateados com um novo recurso ou não concordarem, não há problema em removê-lo. Se eles gostarem, guarde.

Além disso, sempre que lhe pedirem para fazer alguma coisa, vá a "milha extra" e adicione muitas coisas que eles esqueceram de mencionar ou coisas que funcionariam melhor do que o que eles disseram. Mas não pergunte se é "ok" ir a milha extra. Basta fazê-lo e casualmente dizer-lhes sobre isso depois de feito. O que você está fazendo é treiná-los ..

O que acontecerá será que seu gerente o colocará como um "empreendedor" e começará a depositar sua confiança em você, e seus colegas começarão a vê-lo como o líder que você está começando a ser o proprietário do programa. E então, quando coisas como o que você menciona acontecerem no futuro, você terá mais a dizer, porque você é essencialmente a estrela da equipe e os companheiros de equipe recuarão se você não concordar com eles.

    
por 05.02.2017 / 04:23
fonte
3

A revisão de código atende a vários propósitos. Um que você está obviamente ciente é, " Este código é adequado para um propósito? " Em outras palavras, é funcionalmente correto; é adequadamente testado; são as partes não óbvias devidamente comentadas; está de acordo com as convenções do projeto?

Outra parte da revisão de código é o compartilhamento de conhecimento sobre o sistema. É uma oportunidade para o autor e o revisor aprenderem sobre o código alterado e como ele interage com o resto do sistema.

Um terceiro aspecto é que ele pode fornecer uma revisão dos problemas que existiam antes de quaisquer alterações serem feitas. Muitas vezes, ao revisar as alterações de outra pessoa, localizo algo que perdi em uma iteração anterior (muitas vezes algo de minha autoria). Uma declaração como "Aqui está uma oportunidade para tornar isso mais robusto do que era", não é uma crítica, e não a tome como uma só!

Minha equipe atual considera a revisão de código não apenas como um gateway ou obstáculo que o código deve passar incólume antes de confirmar, mas principalmente como uma oportunidade para uma discussão estruturada de uma área específica de funcionalidade. É uma das ocasiões mais produtivas para o compartilhamento de informações. (E essa é uma boa razão para compartilhar a revisão em torno da equipe, em vez de sempre o mesmo revisor).

Se você perceber as revisões de código como uma atividade de confronto, essa é uma profecia auto-realizável. Se, em vez disso, você considerá-los como a parte mais colaborativa de seu trabalho, eles estimularão melhorias contínuas em seu produto e em como você trabalha em conjunto. Ajuda se uma revisão puder ser clara sobre as prioridades relativas de suas sugestões - há uma diferença entre "Eu gostaria de um comentário útil aqui" e "Isso quebra se x for negativo", por exemplo.

Tendo feito um monte de declarações gerais acima, como isso se aplica à sua situação específica? Espero que agora seja óbvio que meu conselho é responder à revisão com perguntas abertas e negociar qual abordagem é mais valiosa. Para o seu caso de exemplo em que um teste extra é sugerido, algo como: "Sim, podemos testar isso; eu estimo que ele levará para ser implementado. Você acha que o benefício vale E existe outra coisa que podemos fazer para garantir que o teste não seja necessário? "

Uma coisa me impressiona quando leio sua pergunta: se é necessário um esforço de dois dias para escrever um novo caso de teste, então seu novo teste é um cenário muito diferente dos seus testes existentes (caso em que provavelmente ele muito valor) ou você identificou a necessidade de reutilização de código aprimorada no conjunto de testes.

Finalmente, como um comentário geral sobre o valor das revisões de código (e como um resumo do que eu disse acima), eu gosto desta declaração, em Maintainers não são dimensionados por Daniel Vetter :

At least for me, review isn’t just about ensuring good code quality, but also about diffusing knowledge and improving understanding. At first there’s maybe one person, the author (and that’s not a given), understanding the code. After good review there should be at least two people who fully understand it, including corner cases.

    
por 06.02.2017 / 11:51
fonte
3

O código pode SEMPRE ser melhor.

Se você está em uma revisão de código e não vê nada que possa ser melhor ou um teste de unidade que possa detectar um bug, não é o código perfeito, mas o revisor que não está fazendo o trabalho deles. Se você escolheu mencionar a melhoria é uma escolha pessoal. Mas quase sempre que sua equipe faz uma revisão de código, deve haver coisas que alguém perceba que poderiam ser melhores ou que todos provavelmente perderiam seu tempo.

Dito isto, se você age com base nos comentários ou não, depende da sua equipe. Se suas alterações corrigirem o problema ou adicionarem valor suficiente sem alteração que sua equipe as aceita, junte-as e registre seus comentários no backlog para que alguém o discuta posteriormente. Se a equipe descobrir que suas alterações adicionam mais risco ou complexidade do que o valor, será necessário resolver os problemas de acordo.

Lembre-se de que qualquer código tem pelo menos mais um caso extremo que poderia ser testado e poderia usar pelo menos mais uma refatoração. É por isso que as revisões de código são feitas melhor como um grupo, com todos olhando para o mesmo código ao mesmo tempo. Para que no final todos possam chegar a um consenso sobre se o código em revisão é aceitável (como é) e agrega valor suficiente para se fundir à base da comunidade ou se certas coisas devem ser feitas antes que haja valor suficiente para mesclar .

Como você está fazendo esta pergunta, presumo que você não está realmente fazendo "revisões de código", mas sim criando uma solicitação pull ou outro mecanismo de envio para que outros possam comentar de maneira não determinística. Então agora você está em um problema de gerenciamento e uma definição de feito. Eu acho que o seu gerenciamento é indeciso e não entende o processo e propósito das revisões de código e provavelmente não tem uma "definição de feito" (DOD). Porque se eles fizessem o seu DOD explicitamente responderia a esta pergunta e você não teria que vir aqui e perguntou.

Como você conserta isso? Bem, peça a seu gerente que lhe dê um DOD e diga se você precisa sempre implementar todos os comentários. Se a pessoa em questão é seu gerente, então a resposta é evidente.

    
por 06.02.2017 / 05:33
fonte
3

Esta questão não é sobre as virtudes da programação defensiva, os perigos de casos específicos ou os riscos catastróficos de erros em produtos físicos. Na verdade, nem é mesmo sobre engenharia de software .

O que realmente importa é como um praticante júnior lida com uma instrução de um praticante sênior quando o junior não pode concordar ou apreciá-lo.

Há duas coisas que você precisa apreciar sobre ser um desenvolvedor júnior. Em primeiro lugar, significa que, embora seja possível que você esteja certo, e ele esteja errado, é - no balanço das probabilidades - improvável. Se o seu colega de trabalho está fazendo uma sugestão de que você não pode ver o valor de, você precisa considerar seriamente a possibilidade de não ter experiência suficiente para compreendê-lo. Eu não entendo esse sentido deste post.

A segunda coisa a apreciar é que o seu sócio sênior é chamado porque ele tem mais responsabilidade. Se um júnior quebra alguma coisa importante, eles não ficarão em apuros se seguirem as instruções. Se um senior

permitisse que eles o quebrassem, no entanto - por não levantar questões na revisão de código, por exemplo - eles certamente ficariam muito preocupados.

Em última análise, é um requisito implícito do seu trabalho que você observe as instruções daqueles em que a empresa confia para liderar seus projetos. Você é geralmente incapaz de se submeter a idosos quando há bons motivos para valorizar sua experiência? Você pretende seguir nenhuma instrução que não possa entender? Você acha que o mundo deveria parar até que você esteja convencido? Esses valores são incompatíveis com o trabalho em equipe.

Um último ponto. Pense nos projetos que você escreveu há seis meses. Pense nos projetos que você escreveu na universidade. Veja o quão ruim eles parecem agora - todos os bugs e design de cabeça para baixo, os pontos cegos e as abstrações equivocadas? E se eu dissesse que daqui a seis meses você contará as mesmas falhas no trabalho que está fazendo hoje? Isso ajuda a demonstrar quantos pontos cegos estão em sua abordagem atual? Porque essa é a diferença que a experiência faz.

    
por 07.02.2017 / 18:47
fonte
2

constantly makes comments to my code that suggest me to do a lot more work than is necessary.

Você pode dar recomendações, mas, no final, não cabe a você decidir o que é necessário. O seu trabalho é implementar o que a gerência ou (neste caso, o seu revisor) decidir que é necessário. Se você não concordar com o que é necessário demais ou com muita força, você provavelmente se encontrará fora de um emprego. Consequentemente, faz parte do seu profissionalismo chegar a um acordo com isso e estar em paz com ele.

He had suggested that I handle an obscure case that is extremely unlikely to happen

Há outras ótimas respostas aqui que mostram como até mesmo os bugs (ou seja, algo que provavelmente não pode falhar ever ) ainda deve ser re-trabalhado algumas vezes. (por exemplo, no caso de construção na futura segurança do produto, seguindo padrões, etc.) Parte do papel de um grande desenvolvedor é ter tanta confiança quanto possível que seu código será robusto em todas as situações concebíveis a cada vez, e no futuro. -impedido também, não apenas trabalhando sob situações testadas nas condições atuais na maioria das vezes

    
por 06.02.2017 / 15:18
fonte
2

Sugestões para codificar revisores para aumentar a utilidade comercial de sua revisão de código (você, como OP, deve propor essa mudança):

  • Marque seus comentários por tipo. "Critical" / "Must-Do" / "Opcional" / "Melhorias sugeridas" / "bom ter" / "Estou meditando".

    Se isso parecer muito CDO / anal / complicado, use pelo menos dois níveis: "Deve corrigir para passar a revisão e ter permissão para mesclar sua alteração" / "Todos os outros".

Sugestões para lidar com sugestões de revisão de código que parecem menos importantes:

  • Crie um ticket aberto em seu sistema de bilheteria escolhido (sua equipe usa um, esperançosamente?), rastreando a sugestão

  • Coloque o ticket # como um comentário de resposta ao item de revisão de código se o seu processo permitir respostas a comentários como Fisheye ou revisões por e-mail.

  • Entre em contato com o revisor e pergunte explicitamente se esse item é do tipo "deve corrigir ou não ser mesclado / liberado".

    • Se a resposta for "Sim", mas você não concordar, deixe que a pessoa responsável pelo gerenciamento do projeto (PM, seu gerente, etc ...) tome uma decisão - apresente a discordância completa e honestamente. Isso não é sobre qual de vocês está "certo", mas sobre o que é melhor para o projeto, então o trabalho do gerente / gerente de projetos.

Agora, trate esse ticket como qualquer outra solicitação de desenvolvimento.

  1. Se for decidido ser urgente após o encaminhamento, trate-o como qualquer solicitação de desenvolvimento urgente. Desprioretizar outros trabalhos e trabalhar sobre isso.

  2. Caso contrário, trabalhe de acordo com a prioridade que foi atribuída e seu ROI (que pode diferir com base em sua linha de negócios, conforme explicado em outras respostas).

por 06.02.2017 / 21:21
fonte
2

Você deve não encaminhar para o gerenciamento.

Na maioria das empresas, o indivíduo de gerenciamento sempre escolheu não escrever esse teste extra, para não perder tempo melhorando marginalmente a qualidade do código, para não perder tempo refatorando. / p>

Em muitos casos, a qualidade do código depende das regras não escritas na equipe de desenvolvimento e do esforço extra que os programadores enviam.

Você é um desenvolvedor júnior e esta é sua primeira análise de código . Você deve aceitar o conselho e fazer o trabalho. Você só pode melhorar o fluxo de trabalho e as regras da sua equipe se você os conhecer e respeitar pela primeira vez por um tempo, para que você possa entender por que eles estão lá. Caso contrário, você será aquele cara novo que não respeita as regras e se torna o lobo solitário da equipe.

Você é novo no time, segue os conselhos que recebe por um tempo, descobre por que eles estão lá, não traga o primeiro conselho que você questiona na reunião do scrum. As verdadeiras prioridades do negócio serão evidentes para você depois de um tempo sem perguntar (e pode não ser o que o gerente vai lhe dizer cara a cara).

    
por 07.02.2017 / 12:22
fonte
1

É muito importante que você crie um código que satisfaça sua solicitação de lead / gerenciamento. Qualquer outro detalhe seria apenas um "bom ter". Se você é um especialista (leia-se: "se você não é um desenvolvedor júnior") em seu campo, então você está "qualificado" para tratar de questões menores que você encontra ao longo do caminho sem consultar seus líderes todas as vezes.

Se você acha que algo está errado e você é relativamente experiente em seu campo, as chances são altas de que você esteja certo .

Aqui estão algumas declarações que podem ser úteis para você:

  • Fui solicitado a fazer X, o revisor de código sugere também fazer Y, devo fazê-lo ou devo passar para coisas mais importantes?
  • Você está sugerindo Y para mim, então você pode descobrir pelo menos um caso de teste que detectaria esse comportamento para que eu pudesse testá-lo? Eu acredito que o código não será chamado.
  • Talvez não devêssemos desenvolver um substituto seguro para casos descobertos primeiro? Então nós os pegamos cedo e corrigimos em movimento para que possamos nos concentrar em coisas mais importantes.
  • Ok, enquanto eu estou implementando o Y, você poderia ser tão gentil em escrever alguns casos de teste para isso? ?
  • Fui solicitado a fazer X e acho que poderia fazer Y a menos que houvesse outras prioridades. Da próxima vez, por que você não arquiva uma solicitação de recurso em vez de colocá-la como um comentário de revisão no meu código ? Pelo menos podemos ouvir outras opiniões de outros membros da equipe sobre esse recurso antes de implementá-lo (geralmente qualquer material importante deve ser um recurso e deve ser tratado por mais de uma pessoa; geralmente, revisão de código deve ser sobre revisão código e soluções abordagens, o resto é a correção de bugs e recursos).

Você pensa seriamente que a atitude do seu revisor está danificando o projeto ou você acha que ele está certo na maioria das vezes (exceto talvez às vezes ele comete pequenos erros na avaliação e exagera isso)?

Para mim, parece mais que ele está escrevendo coisas que não pertencem a uma revisão de código, o que é uma prática ruim, porque torna mais difícil para todo mundo rastrear coisas. No entanto, não sei quais comentários ele fez, por isso não posso dizer nada sobre outros casos.

Em geral, tente evitar os dois itens a seguir:

  • Você não está fazendo o que foi solicitado
  • Você faz seu revisor se sentir bobo

Se ele realmente está revisando seu código, é porque a gerência confia nele mais do que você.

    
por 06.02.2017 / 14:25
fonte
-1

Quando há discordância durante a revisão de código sobre o escopo:

  1. Documente o escopo que é realmente coberto pelo código. Ninguém gosta de surpresas desagradáveis.
  2. Perceba que o escopo é uma decisão de negócios. O escopo já deve ser conhecido quando você começar a trabalhar em um recurso, mas se não for, você pode pedir esclarecimentos mais tarde.

Se o revisor de código é quem toma as decisões de negócios, ele pode alterar o escopo a qualquer momento, mesmo durante a revisão de código, mas ele não o faz em sua função como revisor de código.

    
por 10.02.2017 / 19:05
fonte
-1

Se você não puder provar que o caso de borda é impossível, você deve assumir que isso seja possível. Se for possível, então é inevitável que acabe ocorrendo e, mais cedo ou mais tarde. Se o caso de borda não tiver ocorrido no teste, isso pode indicar que a cobertura de teste está incompleta.

  1. Aceite o feedback.
  2. Antes de fazer qualquer alteração em seu código, faça o melhor para criar um teste para o caso de borda e veja se você pode obter uma falha de teste (prova de que o problema existe). Se for impossível criar um caso de teste desse tipo e obter uma falha de teste, você poderá concluir que o caso de borda é realmente impossível (embora eu hesitaria em chegar a essa conclusão) .
  3. Se você conseguir uma falha de teste, aplique a alteração de código apropriada para passar no teste.

Para o bem do produto, você provavelmente quer ser capaz de produzir o caso de borda e induzir uma falha, para que possa aplicar a correção e ter certeza de que um possível problema foi evitado.

O ponto principal das revisões de código é dar mais atenção ao código. Nenhum de nós está imune a erros ou omissões. É muito comum olhar para um pedaço de código muitas vezes e não perceber um erro óbvio, em que um novo par de olhos pode captá-lo imediatamente.

Como você disse, você implementou um novo algoritmo. Não seria sensato tirar conclusões sobre o comportamento do seu novo algoritmo com base no comportamento ou nas observações sobre o seu antecessor.

    
por 13.02.2017 / 00:31
fonte
-2

Existem revisores de código que sabem o que estão fazendo e, se disserem que algo precisa ser alterado, precisam ser alterados e, quando dizem que algo precisa ser testado, ele precisa ser testado.

Existem revisores de código que precisam justificar sua própria existência criando um trabalho inútil para outras pessoas.

Qual é o que é para você decidir, e como lidar com o segundo tipo é mais uma questão para o local de trabalho.

Se você usa scrum, então a questão é se o seu trabalho faz o que é suposto fazer (aparentemente funciona), e você pode colocar o tratamento do caso extremamente raro e talvez impossível no backlog, onde ele será priorizado, e se entrar em um sprint, o revisor poderá se sentir à vontade para buscá-lo e fazer o trabalho de 13 horas. Se você fizer o trabalho X e fizer o trabalho X, perceberá que o trabalho Y também precisa ser executado, então o trabalho Y não se torna parte do trabalho X, é seu próprio trabalho independente.

    
por 05.02.2017 / 21:07
fonte