Minha empresa recentemente começou a fazer revisões de código formalizadas. O processo é o seguinte: você envia para um github, solicita uma solicitação pull, o código é revisado por aproximadamente três pessoas e, se tudo for aprovado, seu código será inserido.
O processo parece justo, no entanto, as três pessoas que fazem as revisões de código não parecem ser justas. Percebo que, quando coloco meu código para revisão, recebo algo entre 100 e 200 comentários. O número principal para mim foi de 300 comentários uma vez. Claro que você acha que são grandes mudanças, mas isso pode ser muito pequenas, com menos de 50 linhas de código (que inclui testes de unidade). Todos os comentários são considerados "obrigatórios" e sem argumentos.
Com isso em mente, meu principal problema aqui é que parece um pouco excessivo. Conversei com o grupo e eles me disseram basicamente que só porque eu tinha tantos anos de desenvolvimento em php não significa que eu sou um "desenvolvedor". Claro que isso parece mais doloroso do que não. Também noto que, dentro do grupo, eles não parecem produzir tantos comentários e na maioria das vezes ignoram ou desconsideram outros comentários ou sugestões que raramente o aceitam como um ponto válido, mesmo que algo seja quebrado.
Então, minha pergunta é se isso é justo? Ou comum?
fonte
Respostas:
IMHO essa é a verdadeira questão, uma vez que não há priorização nisso. Quando você recebe de 100 a 300 comentários, deve haver alguns deles com prioridade A (bugs reais), alguns com o prio B (com probabilidade de levar a erros mais tarde) e alguns com o prio C (tudo mais). Diga a seus colegas que você está disposto a respeitar todos os seus desejos, mas para que as mudanças sejam efetivas e seu tempo seja limitado, insista em uma prioridade. Em seguida, comece com a correção do comentário do prio A primeiro e, se você realmente tiver tempo para mais, poderá começar com B (se tiver sorte, seu chefe entenderá que a fixação do prio B e C não é tão importante e fornecerá a você algumas tarefas mais importantes em vez de desperdiçar seu tempo).
fonte
Revisões de código podem ser um processo divisivo.
Você está em um cruzamento importante, no entanto. Faça uma análise cuidadosa sobre sua revisão. Eles estão identificando questões difíceis ou destacando falhas sérias no seu estilo e lógica?
Se for o primeiro, recomendo trabalhar em direção a uma resolução (novo trabalho ou novos processos de revisão de código).
Se for o último, recomendo ler e estudar muito o código para tentar melhorar o seu código com qualidade profissional.
fonte
Parece pelos seus comentários que seus colegas estão usando o processo de revisão de código para concordar com uma metodologia ou aprimorar o código. Comecei a fazer análises de código como você e percebo que às vezes discutimos muito sobre coisas que são apenas abordagens ou melhorias de implementação. Isso não é nada ruim, na medida do razoável (300 comentários parecem muitos para mim, que devem parecer um tópico do reddit)
Talvez você precise concordar com algumas decisões de arquitetura sobre o código antes de começar a implementá-lo ou talvez esteja apenas concordando sobre convenções de nomes, padrões e boas práticas para que todos saibam o que é considerado "bom código".
Se você está em conformidade com seus padrões de código, como você diz, e o código funciona como pretendido, não deve haver muitos comentários; portanto, eles estão usando seu código como um fórum ou estão trollando você como parece que está apontando.
Eu tentaria ser crítico comigo mesmo, tentaria participar das conversas e ver o motivo de todos esses comentários e talvez conversar com eles sobre isso de uma maneira construtiva para ver por que eles estão tão descontentes com seu código e se você pode código de uma maneira que deixe todos felizes e o trabalho não fique parado na revisão de código.
Acabei de ler seus últimos comentários, às vezes, quando você não concorda com o código, pode revisá-lo centenas de vezes e propor alterações em todos os lugares que não o deixam feliz, porque a verdadeira razão é que você teria tomado uma decisão arquitetural diferente e você simplesmente não gosta desse código, não importa quantas vezes você o refatorar. Como eu disse acima, talvez você precise concordar com a abordagem do código com antecedência, portanto, ao escrevê-lo, você sabe o que eles esperam dele e, portanto, seu código seria mais razoável para eles.
fonte
Pelo que você está dizendo, parece-me que eles podem ter um viés contra os desenvolvedores de php e, portanto, eles estão tentando encontrar tudo o que há de errado com o seu código para provar seu ponto de vista.¹
Com relação à própria revisão de código, acredito que, como você já disse, que uma quantidade tão grande de comentários menores é menos útil do que algumas críticas boas e válidas. E embora eu tenha uma experiência limitada com relação às revisões de código, a técnica a seguir funcionou bem para as equipes em que trabalhei no passado.
Além disso, devo dizer que minhas primeiras revisões reais de código também continham mais comentários do que eu esperava inicialmente. No entanto, eu nunca considerei isso ruim. Se você continuar aprendendo com os comentários deles² e estiver disposto a aplicar essas técnicas / melhores práticas recém-aprendidas em seus futuros envios de código, os comentários deverão se tornar menos. Com certeza foi o meu caso ;-)
¹ Na minha experiência, isso acontece muito, como muitos programadores afirmam que php é o mais linguagem de programação mal, tendo os programadores mais inexperientes usá-lo. Eu me distancio dessa afirmação, pois acredito que um ótimo software pode ser escrito em qualquer idioma!
² Supondo que, embora os comentários sejam excessivos, há algum valor neles
fonte
É comum alguém receber mais de 100 comentários em suas revisões de código rotineiramente? Eu diria que não. É comum que pessoas cuja qualidade de código "deixe muito a desejar" recebam muitos comentários, absolutamente.
No entanto, isso também depende das "regras" do processo de revisão de código. Todo mundo tem suas próprias idéias sobre como algo deveria ter sido feito. Se o seu processo de revisão de código permitir que os comentários tenham o formato "Você deve fazer desta maneira, e não dessa maneira", provavelmente receberá MUITOS comentários, mesmo para o código adequado. Se seu processo se destina a encontrar "defeitos", o número de comentários deve ser muito menor.
Na minha experiência, as revisões que permitem "sugestões" para métodos alternativos são uma perda de tempo. Essas "sugestões" devem ser tratadas uma a uma fora do processo de revisão. As revisões de defeitos são mais úteis, pois levam as pessoas a se concentrarem nos bugs, em vez de "por que você não fez o que eu faria?". Também é mais útil porque não há como negar um bug se alguém encontrar um. Portanto, não há sentimentos feridos, mas provavelmente gratidão.
ATUALIZAÇÃO: Com tudo isso dito, algum código é simplesmente ruim, mesmo sem defeitos. Nesse caso, o comentário da revisão deve ser um único comentário que diz algo parecido. "Este código precisa ser limpo. Adie a revisão até que o código seja discutido com [seu nome aqui]." Nesse caso, a revisão adicional do código deve parar até que o comentário seja retificado.
UPDATE2: @ Usuário: Você discute seu código / design com um deles enquanto o desenvolve, para poder implementar o que eles estão procurando antes de chegar ao ponto de fazê-lo do seu jeito? Você está mudando alguma coisa sobre como está desenvolvendo o código com base nas sugestões deles ou continua pensando que está bem? Você está aprendendo alguma coisa com os comentários deles?
Quando sou o líder de um projeto, é meu trabalho ser responsável por TODOS os produtos de trabalho. Se eu aprovar um produto de trabalho, estou reivindicando que o produto é aceitável. Quero ter uma reputação por criar produtos de qualidade. Portanto, tenho expectativas e não aceitarei menos que satisfatório. Ao mesmo tempo, tento ensinar e explicar os motivos de minhas preferências. Essas preferências podem nem sempre ser ideais (principalmente aos olhos dos outros), mas a maioria dessas preferências vem da experiência. Geralmente uma reação para evitar repetir os ruins. Portanto, existem alguns dos meus "defensores" pessoais que são necessários para obter minha aprovação, independentemente da resposta.
Por outro lado, você precisa aprender as expectativas necessárias para obter seus produtos de trabalho aprovados. Você pode discordar, mas como não parece ter autoridade para dominar, aprenda o que é esperado. Duvido que a equipe esteja tentando fazer você falhar. Como isso os faz parecer ruins também. Nesse sentido, apenas demonstre que você está ansioso para aprender (mesmo que não esteja), aceite o que eles dizem e faça o possível para se adaptar às preferências deles, e você provavelmente os verá recuar um pouco. Talvez encontre o que você pode pelo menos tolerar e veja se eles vão dar um pouco de mãos dadas para ensinar seus caminhos. Quem sabe, no processo, você pode aprender algo que realmente poderia levar suas habilidades para o próximo nível.
fonte
Algumas diferenças importantes com o processo de inspeção da equipe:
fonte
Por 50 LOC 300, os comentários parecem um pouco excessivos e - uau - 3 revisores para cada solicitação de recebimento? Sua empresa deve ter muitos recursos.
Da minha experiência para um processo útil de revisão de código, deve haver algumas regras e / ou diretrizes:
Se você não obtiver prioridade dos revisores, pergunte ao gerente de projeto / líder de equipe responsável; a pessoa responsável pelos custos deve ter uma opinião sobre as prioridades.
Se você tiver uma arquitetura definida, um entendimento comum de quais padrões de design você usa em seu projeto e um estilo de código acordado, os comentários da revisão devem ser apenas sobre "problemas reais", como problemas de segurança, bugs não intencionais, casos de canto não cobertos pelo especificado arquitetura etc.
Se sua equipe de desenvolvimento não concordou com "questões de sabor" (por exemplo, se uma variável de membro começar com "m_"), todo revisor forçará você a seguir seu estilo, que é apenas uma perda de tempo / dinheiro.
fonte
Isso realmente me parece um problema de comunicação. Você espera que seu código não seja ruim o suficiente para merecer 300 comentários. Os revisores parecem pensar que você precisa de muito feedback. Discutir de um modo assíncrono para frente e para trás vai desperdiçar muito tempo. Caramba, escrever 300 comentários é uma tremenda perda de tempo. Se esses nem todos são defeitos, é possível, como novo membro da equipe, que você ainda não conheça o estilo da equipe. Isso é normal e algo que deve ser aprendido para acelerar toda a equipe.
Minha sugestão é economizar tempo. Acelere o feedback. Eu gostaria:
As pessoas podem argumentar contra o emparelhamento porque "levará mais tempo", mas obviamente isso não é um problema aqui.
fonte