Qual é a melhor maneira de revisar um código antes que ele seja confirmado no tronco? (SVN)

14

Qual é a melhor maneira de revisar um código antes que ele seja confirmado no tronco SVN? Uma idéia em que estou pensando é que o desenvolvedor comprometa seu código em uma ramificação e revise seu código enquanto mescla as revisões da ramificação no tronco. Esta é uma boa prática? Caso contrário, o que mais poderia ser feito para revisar um código antes que ele seja confirmado no tronco?

Meysam
fonte
2
Você pode procurar em algumas ferramentas, como o Crisol, que fornecem suporte para revisões de pré-confirmação.
Gyan aka Gary Buyn
3
algum motivo para não revisar um código depois que ele foi confirmado no tronco?
Gnat #
3
@gnat Bem, acho melhor que os desenvolvedores juniores comprometam seu código em outro lugar e depois mesclem essas alterações no tronco por um desenvolvedor sênior, depois que ele as revise e garanta que essas alterações sejam boas para o tronco. Você sabe, um desenvolvedor sênior, depois de revisar o código confirmado diretamente no tronco, pode decidir que esse código tenha problemas, pois deve ser revertido. Poderíamos ter impedido que esse código problemático fosse confirmado no tronco em primeiro lugar. Essa é a ideia toda.
Meysam 16/04
você já tentou de outra maneira ou isso é apenas suposição?
Gnat #

Respostas:

12

No entanto, existem duas escolas - o que você propõe ou "revisa antes de confirmar". A maioria das diferenças pode ser vista como negativa e / ou positiva. - por exemplo, nenhum rastreamento de alterações causadas por uma revisão - você deseja vê-las como confirmações discretas ou está interessado apenas no trabalho final?

Revise antes da confirmação - nenhuma ramificação necessária (embora possa ser feita, se desejado), deve fornecer aos revisores acesso às pastas de trabalho. O código pode ser alterado durante e após a revisão sem rastreamento. As correções causadas pela revisão não aparecem no repositório.

Revisar após confirmação (em uma ramificação) - é necessário girar uma ramificação para cada revisão (embora isso já possa estar no fluxo de trabalho). O código enviado para revisão não pode ser alterado sem o rastreamento das alterações. Alguém tem que mesclar as ramificações revisadas e acompanhar o que foi revisto e o que não foi.

Depende em grande parte da cultura e experiência da equipe. Em que você confia no modelo, qual é o principal objetivo das revisões? Pessoalmente, prefiro revisar após confirmar, pois ele permite que as alterações resultantes da revisão sejam rastreadas. Agora usamos Git e Gerrit, pois eles fornecem um bom equilíbrio entre as duas opções.

mattnz
fonte
1
A criação constante de ramificações e a fusão repetida é uma desvantagem que supera em muito o tronco potencialmente (e irrevogavelmente) poluente. Geralmente, a principal diretiva para controle de versão é "não quebre a compilação". Se você pode fazer isso, não há nenhum dano real no check-in, e tudo o resto é apenas um ajuste após o fato.
Spencer Kormos
A revisão após a confirmação em uma ramificação funciona bem com o uso de ramificações de recursos: você inicia uma nova ramificação para cada novo recurso ou correção de bug. Quando é concluída e passou na revisão, ela é mesclada ao tronco. Dessa forma, o tronco contém apenas alterações completas e revisadas. Como as ramificações dos recursos têm vida curta, as mesclagens geralmente são triviais. A outra vantagem é que o tronco contém apenas recursos e correções completas - tudo o que estiver meio cozido só existirá em uma ramificação.
Stephen C. Aço
7
  1. Antes de confirmar, execute 'svn diff' para gerar um arquivo de correção.
  2. Envie o arquivo de correção ao revisor que o aplica a uma cópia limpa do tronco.
  3. O revisor examina as alterações usando o visualizador de diferenças de sua escolha.
  4. O revisor executa testes de compilação e executa.
  5. Se tudo der certo, informe ao desenvolvedor que eles podem verificar suas alterações. Se
    houver problemas, o desenvolvedor voltará à etapa 1.
Charles E. Grant
fonte
5

Existe o mundo ideal e depois o mundo real.

No mundo ideal , todo o seu código é testado, para que você possa ter certeza de que qualquer coisa que seja registrada funcione ou você saberá que está quebrado porque falha em um ou mais testes. Além disso, qualquer pessoa que não seja tão experiente será emparelhada com alguém que é experiente, portanto a revisão do código é feita on-the-fly (você se compromete).

No mundo real , as coisas são diferentes. As empresas querem que essa mudança viva agorae dirá a você, com uma cara perfeitamente séria, que sim, você terá tempo para limpar o código e adicionar casos de teste mais tarde. Você provavelmente não terá tempo para revisar tudo, e a porcentagem de código coberta pelos testes diminui continuamente. A principal razão para a revisão de código é que os desenvolvedores juniores aprendam com os desenvolvedores seniores (quando há tempo para isso) fazendo com que uma pessoa mais experiente examine as mudanças e sugira "melhores maneiras de fazer as coisas (TM)". Você terá desenvolvedores seniores apenas enviando código não revisado. Ramificar apenas para uma revisão de código e depois mesclar é uma enorme perda de tempo. Uma maneira de superar esse problema é declarar uma reunião semanal regular de duas horas (ou mais) em que você escolhe uma ou duas alterações recentes nas quais as pessoas trabalharam em pouco tempo e faz com que seus autores "apresentem" sua abordagem, analisando o código juntos em um projetor ou algo assim. Isso pode levar a algumas discussões interessantes (geralmente sai um pouco do tópico), mas geralmente melhora a compreensão de todos sobre como fazê-lo corretamente. Além disso, a pressão de possivelmente ter que apresentar seu código faz com que algumas pessoas o façam melhor ;-)

Ou você pode ter sorte e começar a trabalhar em um ambiente do mundo real, onde não é tão agitado, os programadores são realmente apreciados pelo que fazem, em vez de abusados, e há tempo para fazer tudo certo. Nesse caso, minha resposta seria: tente alguns dos diferentes métodos sugeridos nas respostas aqui e veja qual deles se encaixa na sua equipe e a maneira como você trabalha melhor.

Amos M. Carpenter
fonte
+1 para a ideia de revisão semanal. Talvez eu tenha que tentar este #
Jamie Taylor
@JamieTaylor: Bem, é um pouco comprometido - obviamente, se você (e sua equipe de desenvolvimento) tiverem tempo, eu recomendaria revisões completas de código. Mas é uma boa maneira de compartilhar conhecimento dentro da equipe.
Amos M. Carpenter
2

As ramificações devem funcionar bem, com base na minha experiência de usá-las em revisões pré-confirmadas em trabalhos anteriores.

Observe que estávamos usando revisões pré-confirmação apenas para patches críticos para o código candidato à liberação de produção, portanto não havia muitas ramificações (as alterações de rotina foram passadas pelas revisões pós-confirmação).

Como você parece usar revisões de pré-confirmação para todas as alterações, pode ser necessário gerenciar uma grande quantidade de ramificações. Se você espera que o desenvolvedor faça uma alteração "passível de revisão" por semana, em média, acabará tendo cerca de 50 ramificações por ano para cada desenvolvedor da equipe. Se você estiver usando pedaços menores de trabalho - como aqueles que levam 1, 2, 3 ... dias - multiplique 50 por 2, 3, 5 ... de acordo.

Abaixo estão algumas outras considerações a serem consideradas, se você desejar da melhor maneira .

1. lidar com casos em que a revisão atrasada bloqueia o código necessário para outros membros da equipe

Estabelecer, monitorar e resolver conflitos relacionados aos prazos de revisão de código. De acordo com minha lembrança de revisões pré-confirmação de alterações de rotina com as quais lidei em um dos projetos anteriores, o prazo razoável é de cerca de 3 dias e o momento para começar a me preocupar é quando a revisão não é concluída mais de um dia após o envio.

Para comparação, nas revisões pós-confirmação, esses requisitos são muito mais relaxados (estou usando o prazo de 2 semanas e começo a me preocupar após 1 semana) - mas como você direciona as revisões pré-confirmação, isso provavelmente não é interessante.

2. mesclar conflitos ao enviar o código revisado

O que fazer se a confirmação do código revisado for bloqueada por alterações conflitantes confirmadas por outra pessoa enquanto o código aguardava a revisão?

Algumas opções a serem consideradas são

  • reverta para o início e exija que os desenvolvedores reimplemente e revejam a mudança.
    Nesse caso, talvez seja necessário abordar um impacto negativo no moral da equipe que isso possa ter (terá!).
  • passe a responsabilidade de mesclar para outro membro da equipe ("mestre de mesclagem").
    Nesse caso, você também precisará decidir se as mesclas em si devem passar por uma revisão pré-confirmação ou não - e, se sim, o que fazer no caso de se essa fusão, por sua vez, encontra outro conflito.
  • ignorar as alterações feitas no código revisado no estágio de mesclagem
    Nesse caso, talvez seja necessário abordar um impacto negativo no moral da equipe relacionado ao fato de o código confirmado diferir daquele que foi revisado.
  • inventar uma maneira de evitar conflitos A
    abordagem direta é permitir que apenas um desenvolvedor de cada vez modifique um conjunto específico de arquivos - embora isso não o proteja contra o tipo de alterações que não modificam os arquivos diretamente, mas os impacta através de alterações internas da API . Você também pode descobrir que esse tipo de "bloqueio pessimista" torna as mudanças em todo o sistema e a refatoração profunda bastante problemáticas.

Para comparação, não haveria problemas desse tipo nas revisões pós-confirmação (já que tratam de código que já foi mesclado por definição) - mas como você direciona as revisões pré-confirmação, isso provavelmente não é interessante.

3. carregar desenvolvedor que está aguardando revisão

Estabeleça uma política explícita para saber se o desenvolvedor que enviou a revisão deve mudar para uma nova tarefa ou fazer outra coisa (como por exemplo, perseguir o revisor).

Para comparação, as revisões pós-confirmação dificilmente precisam de uma política explícita (já que é natural prosseguir para a próxima tarefa depois que você confirma o código e levando em conta que o prazo de revisão é de uma semana ou duas) - mas como você direciona as revisões pré-confirmação, provavelmente isso não é interessante.

mosquito
fonte
0

Qualquer parte do desenvolvimento que precise de uma revisão teria que estar em um ramo separado. Portanto, o ramo já deve existir antes que chegue o momento da revisão. Então a etapa precisaria ser:

  1. Reveja
  2. Revisar (possivelmente)
  3. volta à revisão (possivelmente)
  4. Mesclar no tronco

Mesclar é a parte difícil. Quanto mais tempo o ramo permanecer independente, mais difícil será mesclá-lo de volta ao tronco. (Também pode ser mais difícil de testar.)

A mesclagem cruzada é uma solução possível. Antes de mesclar no tronco (etapa 4, ou mesmo antes, digamos, antes das etapas 3 ou 1), mescle o tronco no ramo. O desenvolvedor pode fazer e testar. Em seguida, o ramo alcança o tronco e fica mais fácil mesclá-lo ao tronco. A fusão no tronco é melhor feita por você ou por quem estiver encarregado do tronco.

Algumas pessoas tentam refazer a reestruturação em vez de mesclar. Algumas pessoas argumentam que rebase é mau. Esse é outro debate.

Uday Reddy
fonte