No final de uma sprint de 2 semanas e uma tarefa possui uma revisão de código, na revisão, descobrimos uma função que funciona, é legível, mas é bastante longa e possui alguns odores de código. Trabalho de refatoração fácil.
Caso contrário, a tarefa se encaixa na definição de concluído.
Temos duas escolhas.
- Falha na revisão do código, para que o ticket não seja fechado neste sprint, e levamos um pouco de moral, porque não podemos passar o ticket.
- O refator é um pequeno pedaço de trabalho e seria realizado no próximo sprint (ou mesmo antes de começar) como uma pequena história de meio ponto.
Minha pergunta é: existem problemas ou considerações inerentes ao levantar uma multa nas costas de uma revisão, em vez de falhar?
Os recursos que posso encontrar e ler as revisões de código de detalhes como 100% ou nada, geralmente, mas acho que isso geralmente não é realista.
agile
code-reviews
Erdrik Ironrose
fonte
fonte
Respostas:
Não inerentemente. Por exemplo, a implementação da mudança atual pode ter descoberto um problema que já estava lá, mas não era conhecido / aparente até agora. Falha no ingresso seria injusto, pois você falharia com isso por algo não relacionado à tarefa realmente descrita.
No entanto, suponho que a função aqui seja algo que foi adicionado pela alteração atual. Nesse caso, o ticket deve falhar, pois o código não passou no teste de cheiro.
Onde você desenharia a linha, se não onde você já a desenhou? Você claramente não acha que esse código é suficientemente limpo para permanecer na base de código em sua forma atual; então por que você consideraria dar um passe para o bilhete?
Parece-me que você está indiretamente argumentando que está tentando dar um passe a esse bilhete para beneficiar o moral da equipe, em vez de beneficiar a qualidade da base de código.
Se for esse o caso, você tem suas prioridades misturadas. O padrão de código limpo não deve ser alterado simplesmente porque deixa a equipe mais feliz. A correção e a limpeza do código não dependem do humor da equipe.
Se a implementação do ticket original causou o cheiro do código, ele deve ser endereçado no ticket original. Você só deve criar um novo ticket se o cheiro do código não puder ser atribuído diretamente ao ticket original (por exemplo, um cenário de "palha que quebrou as costas do camelo").
A aprovação / reprovação é inerentemente um estado binário , que é inerentemente tudo ou nada.
O que você está se referindo aqui, acho, é mais do que interpretar as revisões de código como exigindo código perfeito ou falhando, e esse não é o caso.
O código não deve ser imaculado, deve simplesmente obedecer ao padrão razoável de limpeza que sua equipe / empresa emprega. A adesão a esse padrão é uma escolha binária: ela adere (passa) ou não (falha).
Com base na sua descrição do problema, fica claro que você não acha que isso segue o padrão de código esperado e, portanto, não deve ser passado por outras razões, como o moral da equipe.
Se "ele faz o trabalho" fosse a melhor referência para a qualidade do código, não teríamos que inventar o princípio do código limpo e das boas práticas para começar - o compilador e o teste de unidade já seriam nosso processo de revisão automatizada e você não precisaria de revisões de código ou argumentos de estilo.
fonte
Por que isso aparece no final do sprint? Uma revisão de código deve ocorrer assim que você achar que o código está pronto (ou mesmo antes). Você deve verificar sua definição de concluído com cada história que terminar.
Se você se encontra terminando as histórias tão pouco antes da revisão da demo / sprint que não pode ser uma tarefa "minúscula", precisa melhorar a estimativa do seu trabalho. Sim, essa história não terminou. Não por causa de uma revisão de código, mas porque você não planejou incorporar alterações da revisão de código. É como estimar "testes" para levar tempo zero, porque "se você programou corretamente, funcionará, certo?". Não é assim que funciona. Os testes encontrarão erros e a revisão de código encontrará coisas para mudar. Se não, seria uma grande perda de tempo.
Então, para resumir: sim, o Departamento de Defesa é binário. Passar ou falhar. Uma revisão de código não é binária, deve ser mais uma tarefa em andamento. Você não pode falhar . É um processo e no final está feito. Mas se você não planejar adequadamente, não chegará ao estágio "pronto" a tempo e ficará preso no território "não concluído" no final do sprint. Isso não é bom para o moral, mas você precisa levar isso em conta no planejamento.
fonte
Simples: você revisa a alteração . Você não revisa o estado do programa caso contrário. Se eu corrigir um bug em uma função de 3.000 linhas, verifique se minhas alterações corrigem o erro, e é isso. E se minha alteração corrigir o erro, você aceita a alteração.
Se você acha que a função é muito longa, insere uma solicitação de alteração para torná-la mais curta ou divide-a depois que minha alteração foi aceita, e essa solicitação de alteração pode ser priorizada de acordo com sua importância. Se for decidido que a equipe tem coisas mais importantes a fazer, isso será tratado mais tarde.
Seria ridículo se você pudesse decidir prioridades de desenvolvimento durante uma revisão de código, e rejeitar minha alteração por esse motivo seria uma tentativa de decidir prioridades de desenvolvimento.
Em resumo, é absolutamente aceitável aceitar uma alteração no código e aumentar imediatamente um ticket com base no que você viu ao revisar a alteração. Em alguns casos, você fará isso mesmo se a alteração em si tiver causado os problemas: Se é mais importante fazer as alterações agora do que corrigir os problemas. Por exemplo, se outros foram bloqueados, aguardando a alteração, você deseja desbloqueá-los enquanto o código pode ser aprimorado.
fonte
Este parece ser o problema.
Em teoria, você sabe o que deve fazer, mas é próximo do prazo para não querer fazer o que sabe que deve fazer.
A resposta é simples: faça o que você faria se tivesse o mesmo código para revisão de código no primeiro dia do sprint. Se seria aceitável, então deveria ser agora. Se não, não faria agora.
fonte
Uma grande parte do processo é decidir o que significa feito e aderir às suas armas. Isso também significa não comprometer demais e fazer as revisões por pares a tempo de permitir que os testes garantam que o trabalho também esteja funcionalmente completo.
Quando se trata de problemas de revisão de código, existem algumas maneiras de lidar com isso, e a escolha certa depende de alguns fatores.
Resumindo, quando você terminar o trabalho, precisará fazê-lo. Se houver problemas maiores do que o desenvolvedor trabalhou, levante a bandeira e siga em frente. Mas você não deve estar em uma posição em que haja horas antes do final do sprint e agora está apenas começando a revisão por pares. Isso cheira a comprometer demais os recursos ou a adiar as revisões por pares. (um cheiro de processo).
fonte
Não há problemas inerentes à priorização de problemas de revisão de código, mas parece que os principais problemas com os quais você precisa concordar, como equipe, são:
Tudo isso se resume ao que a equipe concordou como a definição de Concluído. Se passar na revisão de código com Zero Issues for a definição de concluído para um item de trabalho, não será possível fechar um item que não atendeu a esse requisito.
É o mesmo que se durante o teste de unidade um teste de unidade falhou. Você conserta o bug, não ignora o teste de unidade, se passar nos testes de unidade era um requisito para ser Concluído.
Se a equipe não concordar em que as revisões de código sejam uma definição de Concluído, suas revisões de código não serão um teste de aceitação do item de trabalho. Eles são uma atividade da equipe que faz parte do processo de backlog para procurar trabalho adicional que possa ser necessário. Nesse caso, quaisquer problemas que você descobrir não estão relacionados aos requisitos do item de trabalho original e são novos itens de trabalho para a equipe priorizar.
Por exemplo, pode ser completamente aceitável para uma equipe desvalorizar a correção de erros de digitação em alguns nomes de variáveis, pois isso não afeta a funcionalidade de negócios que foi entregue, mesmo que a equipe realmente odeie ver o nome da variável "myObkect".
fonte
As respostas mais votadas aqui são muito boas; este aborda o ângulo de refatoração.
Na maioria dos casos, a maioria do trabalho ao refatorar é entender o código existente; alterá-lo depois disso é geralmente a parte menor do trabalho por um de dois motivos:
Se apenas tornar o código mais claro e / ou conciso, as alterações necessárias são óbvias. Freqüentemente, você entendia o código experimentando alterações que pareciam mais limpas e vendo se realmente funcionavam ou se perdiam alguma sutileza no código mais complexo.
Você já tem em mente um design ou estrutura específica que precisa para facilitar a construção de um novo recurso. Nesse caso, o trabalho para desenvolver esse design fazia parte da história que gerou a necessidade dele; é independente da necessidade de refatoração para chegar a esse design.
Aprender e entender o código existente é uma quantidade razoável de trabalho para um benefício não permanente (daqui a um mês alguém provavelmente terá esquecido muito sobre o código se não continuar lendo ou trabalhando com ele nesse período) e, portanto, não faz sentido fazer isso, exceto em áreas de código que estão causando problemas ou que você planeja alterar em um futuro próximo. Por sua vez, como esse é o principal trabalho de refatoração, você não deve refatorar no código, a menos que esteja causando problemas ou planeje alterá-lo em um futuro próximo.
Mas há uma exceção a isso: se alguém atualmente tem um bom entendimento do código que vazará com o tempo, usar esse entendimento para tornar o código mais claro e mais rapidamente entendido mais tarde pode ser um bom investimento. Essa é a situação em que alguém que acabou de desenvolver uma história está.
Nesse caso, você está pensando em criar uma história separada para refatoração é um sinal de alerta em várias frentes:
Você não está pensando em refatorar como parte da codificação, mas como uma operação separada, o que, por sua vez, aumenta a probabilidade de cair sob pressão.
Você está desenvolvendo um código que será mais trabalhoso para entender na próxima vez que alguém precisar trabalhar com ele, tornando as histórias mais demoradas.
Você pode estar perdendo tempo e energia refatorando coisas das quais não obtém muitos benefícios. (Se uma mudança acontecer muito mais tarde, alguém ainda terá que re-entender o código; de qualquer maneira; isso será combinado com mais eficiência com o trabalho de refatoração. Se uma mudança não acontecer mais tarde, a refatoração não servirá a nenhum objetivo, exceto talvez estético.)
Portanto, a resposta aqui é falhar no item para deixar claro que algo em seu processo falhou (nesse caso, o desenvolvedor ou a equipe não está alocando tempo para revisar e implementar as alterações que saem da revisão) e pedir ao desenvolvedor que continue o trabalho imediatamente no item
Quando você estimar para a próxima iteração, reestime a história existente como qualquer quantidade de trabalho que ainda resta para fazê-la passar na revisão e adicioná-la à sua próxima iteração, mas preservando a estimativa da iteração anterior. Quando a história for concluída no final da próxima iteração, defina a quantidade total histórica de trabalho como a soma das primeira e segunda estimativas para que você saiba quanto trabalho estimado foi realmente colocado nela. Isso ajudará a produzir estimativas mais precisas de histórias semelhantes no futuro no estado atual do seu processo. (Ou seja, não presuma que sua subestima aparente não acontecerá novamente; assuma que isso acontecerá novamente até que você complete com êxito histórias semelhantes e dedique menos trabalho.)
fonte
Estou surpreso com a falta de resposta nas respostas e comentários à noção de "falhar" em uma revisão de código, porque esse não é um conceito com o qual eu pessoalmente conheço. Nem me sentiria confortável com esse conceito ou com alguém da minha equipe usando essa terminologia.
Sua pergunta chama explicitamente "práticas ágeis", então vamos revisitar o manifesto ágil (grifo meu):
Fale com sua equipe. Discuta o código em questão. Avalie os custos e os benefícios e decida - como um grupo coeso de especialistas - se deve refatorar esse código agora, mais tarde ou nunca.
Comece a colaborar. Pare de falhar nas revisões de código.
fonte
na revisão, descobrimos uma função que funciona, é legível, mas é bastante longa e tem alguns odores de código ...
Existem problemas ou considerações inerentes ao aumento de um ticket na parte de trás de uma revisão, em vez de falhar?
Não há problema algum (na opinião da minha equipe). Presumo que o código atenda aos critérios de aceitação estabelecidos no ticket (ou seja, ele funciona). Crie um item de lista pendente para abordar o comprimento e qualquer código cheira e priorize-o como qualquer outro ticket. Se for realmente pequeno, basta priorizá-lo alto para o próximo sprint.
Um dos dizeres que temos é "Escolha a melhoria progressiva sobre a perfeição adiada".
Temos um processo muito fluido e construímos um número razoavelmente bom de recursos de 'prova de conceito' (1 ou 2 por sprint) que passam pelo desenvolvimento e pelo teste, mas nunca passam pela revisão interna das partes interessadas (hmm, podemos fazer isso? ?), alfa ou beta ... alguns sobrevivem, outros não.
No projeto atual, perdi a noção de quantas vezes criamos um determinado recurso, o colocamos nas mãos das partes interessadas e um sprint ou dois depois o removi totalmente porque a direção do produto mudou ou os requisitos causados uma reformulação completa de como o recurso deve ser implementado. Todas as tarefas de 'refinamento' restantes para um recurso excluído ou que não se encaixam nos novos requisitos são excluídas, além de fazer parte da preparação da lista de pendências.
fonte
Existem duas maneiras de analisar esse problema na minha opinião:
Academicamente falando, a maioria dos processos de revisão de código existe para falhar na implantação de um PBI (item de lista de pendências do produto) quando o padrão de qualidade do código não é atendido.
No entanto, ninguém no mundo real segue o Ágil ao T, pois, por um (de muitos motivos), diferentes setores têm requisitos diferentes. Assim, fixar o código agora ou assumir dívidas técnicas (você provavelmente criaria um novo PBI) deve ser decidido caso a caso. Se isso comprometer o sprint ou um release ou introduzir uma quantidade razoável de riscos, as partes interessadas da empresa devem estar envolvidas na decisão.
fonte
Nem . Se falhar na revisão do código, a tarefa não será concluída. Mas você não pode deixar de revisar códigos na opinião pessoal. O código passa; vá para a próxima tarefa.
Deve ser uma ligação fácil, e o fato de não ser sugere que você não possui regras escritas suficientemente claras para revisões de código.
"A função é bastante longa". Anote: Funções deve ser inferior a X linhas de comprimento (que estou não sugerindo que as regras sobre o comprimento da função são uma coisa boa).
"Existem alguns odores de código". Anote: as funções públicas devem ter testes de unidade para funcionalidade e desempenho; o uso da CPU e da memória deve estar abaixo dos limites x e y.
Se você não pode quantificar as regras para aprovação de uma revisão de código, receberá esses casos do que é basicamente 'código que você não gosta'.
Você deve falhar com 'código que não gosta'? Eu diria que não. Você começará naturalmente a passar / falhar com base em aspectos que não são de código: Você gosta da pessoa? Eles argumentam fortemente pelo seu caso ou apenas fazem o que lhes é dito? Eles passam seu código quando o revisam?
Além disso, você adiciona uma etapa não quantificável ao processo de estimativa. Estimo uma tarefa com base em como acho que deve ser programada, mas, no final, tenho que mudar o estilo de codificação.
Quanto tempo isso vai adicionar? O mesmo revisor fará a revisão subsequente do código e concordará com o primeiro revisor ou encontrará alterações adicionais? E se eu discordar da mudança e adiar enquanto procuro uma segunda opinião ou discuto o caso?
Se você deseja que as tarefas sejam executadas rapidamente, é necessário torná-las o mais específicas possível. Adicionar um portão de qualidade vaga não ajudará sua produtividade.
Re: É impossível escrever as regras!
Não é tão difícil assim. Você realmente quer dizer "Não consigo expressar o que quero dizer com código" bom "" . Depois de reconhecer isso, você pode ver que obviamente é um problema de RH se começar a dizer que o trabalho de alguém não é o ideal, mas não sabe por quê.
Anote as regras que puder e tenha discussões sobre cerveja sobre o que torna o código 'bom'.
fonte