Pratiche agili: riesame del codice: non riescono a riesaminare o sollevare un problema?

are there any inherent problems or considerations with raising a ticket off of the back of a review, instead of failing it?

Non intrinsecamente. Ad esempio, l'implementazione del cambiamento attuale potrebbe aver riportato alla luce un problema che era già presente, ma non era noto / apparente fino ad ora. Fallire il ticket sarebbe ingiusto, in quanto si fallirebbe per qualcosa non correlato all'attività effettivamente descritta.

in the review we discover a function

Tuttavia, suppongo che la funzione qui sia qualcosa che è stata aggiunta dal cambiamento corrente. In questo caso, il ticket dovrebbe essere fallito in quanto il codice non ha superato il test dell'odore.

Dove disegneresti la linea, se non dove l'hai già disegnata? Chiaramente non pensi che questo codice sia sufficientemente pulito per rimanere nella base di codice nella sua forma attuale; quindi perché dovresti quindi considerare di dare un biglietto al pass?

Fail the code review, so that the ticket doesn't close in this sprint, and we take a little hit on morale, because we cannot pass off the ticket.

Mi sembra che tu stia discutendo indirettamente che stai cercando di dare a questo ticket un pass per beneficiare del morale della squadra, piuttosto che beneficiare della qualità della base di codice.

Se questo è il caso, allora hai le tue priorità mescolate. Lo standard del codice pulito non dovrebbe essere modificato semplicemente perché rende la squadra più felice. La correttezza e la pulizia del codice non dipendono dall'umore della squadra.

The refactor is a small piece of work, and would get done in the next sprint (or even before it starts) as a tiny, half point story.

Se l'implementazione del biglietto originale ha causato l'odore del codice, allora dovrebbe essere indirizzato nel biglietto originale. Dovresti creare un nuovo ticket solo se l'odore del codice non può essere attribuito direttamente al biglietto originale (ad esempio, uno scenario "la goccia che fa traboccare il vaso").

The resources I can find and have read detail code reviews as 100% or nothing, usually, but I find that is usually not realistic.

Pass / fail è intrinsecamente uno stato binario , che è inerentemente tutto o niente.

Ciò a cui ti riferisci qui, penso, è più che interpreti le revisioni del codice in quanto richiedono un codice perfetto o altrimenti non lo è, e non è questo il caso.

Il codice non dovrebbe essere immacolato, dovrebbe semplicemente rispettare lo standard ragionevole di pulizia che il tuo team / azienda impiega. L'aderenza a questo standard è una scelta binaria: aderisce (passa) o non lo fa (fallire).

In base alla tua descrizione del problema, è chiaro che non pensi che ciò rispetti lo standard previsto del codice, e quindi non dovrebbe essere approvato per ulteriori motivi come il morale della squadra.

Otherwise the task fits the definition of done.

Se "ha fatto il lavoro" erano il miglior punto di riferimento per la qualità del codice, allora non avremmo dovuto inventare il principio del codice pulito e delle buone pratiche per cominciare - il compilatore e il collaudo delle unità sarebbero già stati i nostri automatizzati processo di revisione e non avresti bisogno di recensioni di codice o argomenti di stile.

At the end of a 2 week sprint and a task has a code review [...] Easy refactor job.

Perché compare alla fine dello sprint? Una revisione del codice dovrebbe accadere non appena pensate che il codice sia stato fatto (o anche prima). Dovresti controllare la tua definizione di fatto con ogni storia che finisci.

Se ti ritrovi a finire le storie così poco prima della tua demo / sprint che non puoi adattare un compito "minuscolo", allora devi essere più bravo a stimare il tuo lavoro. Sì, quella storia non è finita. Non a causa di una revisione del codice, ma perché non hai intenzione di incorporare le modifiche dalla revisione del codice. È come stimare il "testing" in tempo zero, perché "se lo programmate correttamente, funzionerà, giusto?". Questo non è il modo in cui funziona. I test troveranno errori e la revisione del codice troverà cose da cambiare. Se non lo fosse, sarebbe una grande perdita di tempo.

Quindi riassumendo: sì, il DoD è binario. Promosso o bocciato. Una revisione del codice non è binaria, dovrebbe essere più simile a un'attività in corso. Non puoi fallire . È un processo e alla fine è finito. Ma se non si pianifica correttamente, non si arriverà a quella fase "conclusa" nel tempo e si bloccherà nel territorio "non fatto" alla fine dello sprint. Non va bene per il morale, ma è necessario tenerne conto nella pianificazione.

Semplice: rivedi cambia . Non riesci a rivedere lo stato del programma altrimenti. Se correggo un bug in una funzione di 3.000 linee, allora controlli che i miei cambiamenti risolvono il bug, e il gioco è fatto. E se la mia modifica corregge il bug, accetti il cambiamento.

Se ritieni che la funzione sia troppo lunga, metti una richiesta di modifica per rendere la funzione più breve, o dividi, dopo la mia modifica è stata accettata e quella richiesta di modifica può essere priorità in base alla sua importanza. Se viene presa la decisione che la squadra ha cose più importanti da fare, viene gestita in seguito.

Sarebbe ridicolo se potessi decidere le priorità di sviluppo durante una revisione del codice e rifiutare il mio cambiamento per questo motivo sarebbe un tentativo di decidere le priorità di sviluppo.

In sintesi, è assolutamente accettabile accettare un cambio di codice e generare immediatamente un ticket in base alle cose che hai visto durante la revisione della modifica. In alcuni casi lo farai anche se il cambiamento stesso ha causato i problemi: se è più importante avere le modifiche ora che correggere i problemi. Ad esempio se altri sono stati bloccati, in attesa della modifica, quindi si desidera sbloccarli mentre il codice può essere migliorato.

Fail the code review, so that the ticket doesn't close in this sprint, and we take a little hit on morale, because we cannot pass off the ticket.

Questo sembra essere il problema.
In teoria sai cosa dovresti fare, ma è vicino alla scadenza, quindi non vuoi fare ciò che sai che dovresti fare.

La risposta è semplice: fai ciò che vorresti fare se ricevessi lo stesso codice per la revisione del codice il primo giorno dello sprint. Se fosse accettabile, dovrebbe ora. Se non lo fosse, non lo farebbe ora.

Una parte importante del processo è decidere cosa significa fatto e attaccare alle tue pistole. Significa anche non sovraccaricare e fare in modo che le tue peer review siano fatte in tempo per consentire ai test di garantire che il lavoro sia anche funzionalmente completo.

Quando si tratta di problemi di revisione del codice, ci sono alcuni modi per gestirlo, e la scelta giusta dipende da alcuni fattori.

• Puoi solo pulire il codice da solo e far sapere alla persona che cosa hai fatto. Fornisce alcune opportunità di tutoraggio, ma questo dovrebbe essere abbastanza semplice che può essere fatto in pochi minuti.
• Puoi respingerlo con commenti relativi a ciò che è sbagliato. Se la gestione degli errori è scarsa o lo sviluppatore continua a ripetere gli stessi errori, ciò può essere garantito.
• Puoi creare un ticket e incorrere in un debito tecnico. Il biglietto è lì per essere sicuro di pagarlo più tardi. Potrebbe essere che ti trovi in una fase di crisi e nel processo di revisione delle modifiche vedi un problema più grande non direttamente correlato al cambiamento.

La linea di fondo è che quando hai finito il lavoro devi averne bisogno. Se ci sono problemi più grandi di quelli su cui lo sviluppatore ha lavorato, alza la bandiera e vai avanti. Ma non dovresti essere in una posizione in cui ci sono ore prima della fine dello sprint e ora stai per fare una revisione tra pari. Ha l'odore di sovraccaricare le risorse o procrastinare sulle recensioni dei colleghi. (un odore di processo).

Non ci sono problemi inerenti alla ridefinizione dei problemi di revisione del codice, ma sembra che i problemi principali su cui devi accordarti, come squadra, siano:

1. Qual è lo scopo della revisione del tuo codice?
2. In che modo i risultati della revisione del codice si riferiscono alla definizione di Fatto per un elemento di lavoro?
3. Se la revisione del codice si applica come test di gating, quali problemi sono considerati "bloccanti"?

Tutto si riduce a ciò che la squadra ha accettato come definizione di Fatto. Se la revisione del codice di passaggio con Zero Issues è la definizione di fatto per un elemento di lavoro, non è possibile chiudere un elemento che non ha soddisfatto questo requisito.

È lo stesso che se durante il test dell'unità un test dell'unità fallisse. Correggere il bug, non ignorare il test dell'unità, se il superamento dei test di unità era un requisito per essere Fatto.

Se il team non ha accettato di codificare le recensioni come definite da Fine, le tue revisioni del codice non rappresentano un test di accettazione dell'elemento di lavoro. Si tratta di un'attività di gruppo che fa parte del processo di backlog per cercare lavoro aggiuntivo che potrebbe essere necessario. In tal caso, eventuali problemi rilevati non sono correlati ai requisiti dell'elemento di lavoro originale e sono nuovi elementi di lavoro per il team in base alla priorità.

Ad esempio, potrebbe essere completamente accettabile per un team di ridefinire i refusi di correzione in alcuni nomi di variabili in quanto non ha alcun impatto sulla funzionalità aziendale che è stata consegnata, anche se il team odia davvero vedere il nome della variabile "myObkect".

Le risposte più votate qui sono molto buone; questo affronta l'angolo di refactoring.

Nella maggior parte dei casi, la maggior parte del lavoro durante il refactoring è la comprensione del codice esistente; cambiarlo dopo che è generalmente la parte più piccola del lavoro per uno dei due motivi:

1. Se si rende il codice più chiaro e / o conciso, i cambiamenti necessari sono evidenti. Spesso hai acquisito la comprensione del codice provando i cambiamenti che sembravano più puliti e vedendo se effettivamente funzionavano, o se mancavano di sottigliezza nel codice più complesso.

2. Hai già in mente un particolare design o struttura che devi rendere più semplice la creazione di una nuova funzione. In quel caso, il lavoro per sviluppare quel design era parte della storia che ne aveva generato l'esigenza; è indipendente da te che devi eseguire il refactoring per ottenere quel design.

Imparare e comprendere il codice esistente è una buona quantità di lavoro per un beneficio non permanente (tra un mese qualcuno potrebbe aver dimenticato molto del codice se non continuerà a leggere o lavorare con esso per quel periodo), e quindi non ha senso farlo se non su aree di codice che ti causano problemi o che stai pianificando di cambiare nel prossimo futuro. A sua volta, poiché questo è il principale lavoro di refactoring, non dovresti eseguire il refactoring sul codice a meno che non stia causando problemi o stia pianificando di modificarlo nel prossimo futuro.

Ma c'è un'eccezione: se qualcuno attualmente ha una buona conoscenza del codice che colerà nel tempo, usare quella comprensione per rendere il codice più chiaro e più rapidamente compreso in seguito può essere un buon investimento. Questa è la situazione in cui qualcuno ha appena finito di sviluppare una storia.

The refactor is a small piece of work, and would get done in the next sprint (or even before it starts) as a tiny, half point story.

In questo caso, stai pensando di creare una storia separata per il refactoring è un segnale di avvertimento su diversi fronti:

1. Non stai pensando al refactoring come parte della codifica, ma come un'operazione separata, che a sua volta rende probabile che venga rilasciata sotto pressione.

2. Stai sviluppando un codice che sarà più utile per capire la prossima volta che qualcuno dovrà lavorarci, rendendo le storie più lunghe.

3. Potresti sprecare tempo ed energia a rifare le cose dalle quali non ottieni molti benefici. (Se un cambiamento avviene molto dopo, qualcuno dovrà comunque ri-capire il codice, comunque, questo è combinato in modo più efficiente con il lavoro di refactoring. Se un cambiamento non avviene più avanti, il refactoring non ha funzionato scopo, tranne forse uno estetico.)

Quindi la risposta qui è di fallire l'elemento per chiarire che qualcosa nel tuo processo è fallito (in questo caso, è lo sviluppatore o il team che non assegna il tempo per la revisione e l'implementazione delle modifiche che escono dalla revisione) e lo sviluppatore continuare immediatamente a lavorare sull'elemento.

Quando vai a stimare la prossima iterazione, rivaluta la storia esistente come qualsiasi quantità di lavoro sembra essere lasciata per farla passare in rassegna e aggiungerla alla tua prossima iterazione, ma preservandola la stima dalla precedente iterazione. Quando la storia è completata alla fine della successiva iterazione, imposta la quantità totale storica di lavoro sulla somma della prima e della seconda stima in modo da sapere quanto lavoro stimato è stato effettivamente messo in esso. Ciò contribuirà a produrre stime più accurate di storie simili in futuro allo stato attuale del processo. (Ad esempio, non dare per scontato che la sottovalutazione apparente non si verificherà più, supponiamo che accadrà di nuovo fino a quando non avrai completato con successo storie simili mentre lavori meno.)

nella recensione scopriamo una funzione che funziona, è leggibile, ma è piuttosto lunga e ha qualche odore di codice ...

Esistono problemi o considerazioni inerenti l'aumento di un ticket dal retro di una revisione, invece di fallire?

Nessun problema (nell'opinione dei miei team). Presumo che il codice soddisfi i criteri di accettazione indicati nel biglietto (vale a dire, funziona). Crea un elemento di backlog per indirizzare la lunghezza e qualsiasi odore di codice e assegna la priorità come qualsiasi altro ticket. Se è veramente piccolo, basta dare la priorità al prossimo sprint.

Uno dei detti che abbiamo è "Scegli un miglioramento progressivo rispetto alla perfezione posticipata".

Abbiamo un processo molto fluido e costruisco un discreto numero di funzionalità di "proof of concept" (1 o 2 per sprint) che lo fanno attraverso lo sviluppo e il test, ma non superiamo mai la recensione interna degli stakeholder (hmm, possiamo fai questo invece?), alfa o beta ... alcuni sopravvivono, altri no.

Nel progetto corrente, ho perso la cognizione di quante volte abbiamo creato una determinata funzionalità, l'abbiamo messa nelle mani degli stakeholder e uno o due sprint dopo, completamente rimossa perché la direzione del prodotto è cambiata, o i requisiti hanno causato una rifusione completa di come dovrebbe essere implementata la funzionalità. Eventuali rimanenti attività di "perfezionamento" per una funzione eliminata o che non si adattano ai nuovi requisiti vengono eliminate così come parte del grooming del backlog.

Secondo me, ci sono due modi per considerare questo problema:

1. Il modo accademico
2. La via del mondo reale

A livello accademico, la maggior parte dei processi di revisione del codice esiste per fallire nella distribuzione di un PBI (product backlog item) quando lo standard di qualità del codice non viene rispettato.

Tuttavia, nessuno nel mondo reale segue agile la T come per una (per molte ragioni), diverse industrie hanno requisiti diversi. In tal modo, la correzione del codice ora o l'assunzione del debito tecnico (si creerebbe un nuovo PBI molto probabilmente) dovrebbero essere decisi su per caso. Se sta per compromettere lo sprint o un rilascio o introdurre una quantità irragionevole di rischio, le parti interessate dovrebbero essere coinvolte nella decisione.

Sono sorpreso dalla mancanza di risposta nelle risposte e nei commenti alla nozione di "fallimento" di una revisione del codice, perché non è un concetto che, personalmente, conosco. Né sarei a mio agio con quel concetto o con chiunque nella mia squadra che usasse quella terminologia.

La tua domanda richiede esplicitamente "pratiche agili", quindi rivisitiamo il manifesto agile (enfasi mia):

We are uncovering better ways of developing software by doing it and helping others do it. Through this work we have come to value:

• Individuals and interactions over processes and tools
• Working software over comprehensive documentation
• Customer collaboration over contract negotiation
• Responding to change over following a plan

That is, while there is value in the items on the right, we value the items on the left more.

Parla con la tua squadra. Discutere il codice in questione. Valuta i costi e i benefici e decidi - come gruppo coeso di esperti - se riformulare questo codice ora, più tardi o mai più.

Inizia a collaborare. Interrompi la mancata revisione del codice.

Nessuno . Se fallisce la revisione del codice, l'attività non viene eseguita. Ma non si possono fallire le revisioni del codice sull'opinione personale. Il codice passa; vai al prossimo compito.

Dovrebbe essere una chiamata facile, e il fatto che non sia suggerito che tu non abbia regole scritte abbastanza chiare per le revisioni del codice.

1. "La funzione è abbastanza lunga". Scrivi: le funzioni devono essere lunghe meno di X righe (io sono non suggerisco che le regole sulla durata della funzione sono una buona cosa).

2. "Ci sono alcuni odori di codice". Scrivi: le funzioni pubbliche devono avere test unitari per funzionalità e prestazioni, sia l'utilizzo della CPU che della memoria devono essere inferiori ai limiti x e y.

Se non riesci a quantificare le regole per il superamento di una revisione del codice, otterrai questo caso di quello che in pratica è "codice che non ti piace".

Dovresti fallire nel codice che non ti piace? Direi di no Comincerai naturalmente a passare / fallire in base agli aspetti non di codice: ti piace la persona? Litigano strongmente per il loro caso o fanno semplicemente quello che gli viene detto? Passano il tuo codice quando ti esaminano?

Inoltre, si aggiunge un passaggio non quantificabile al processo di stima. Stimo un compito in base a come penso debba essere programmato, ma poi alla fine devo cambiare lo stile di codifica.

Quanto durerà questo? Lo stesso revisore farà la successiva revisione del codice e sarà d'accordo con il primo revisore o troverà ulteriori cambiamenti? Cosa succede se non sono d'accordo con il cambiamento e rimandare a farlo mentre cerco una seconda opinione o discuto il caso?

Se vuoi eseguire rapidamente le attività, devi renderle il più specifiche possibili. Aggiungere un varco di qualità vaga non aiuterà la tua produttività.

Ri: È impossibile scrivere le regole !!

Non è poi così difficile. Intendi veramente "Non posso esprimere ciò che intendo con 'buono' codice" . Una volta riconosciuto, puoi vedere che si tratta ovviamente di un problema di risorse umane se inizi a dire che il lavoro di qualcuno non è all'altezza, ma non puoi dire perché.

Annota le regole che puoi e discuti della birra su ciò che rende il codice "buono".