Qual è il modo più efficace per eseguire revisioni del codice? [chiuso]

Al mio lavoro abbiamo una regola molto semplice: le modifiche devono essere esaminate da almeno un altro sviluppatore prima di un'unione o un commit al trunk . Nel nostro caso ciò significa che l'altra persona si trova fisicamente con te al computer e passa attraverso la lista delle modifiche. Questo non è un sistema perfetto, ma ha nondimeno migliorato la qualità del codice.

Se sai che il tuo codice verrà sottoposto a una revisione che ti costringe a cercarlo prima. Molti problemi diventano evidenti allora. Sotto il nostro sistema, devi spiegare cosa hai fatto al revisore, il che ti fa ancora notare problemi che potresti aver perso prima. Inoltre, se qualcosa nel codice non è immediatamente chiaro al revisore, è una buona indicazione che è richiesto un nome migliore, un commento o un refactoring. E, naturalmente, anche il revisore potrebbe trovare dei problemi. Inoltre, oltre a esaminare la modifica, il revisore potrebbe notare problemi nel codice nelle vicinanze.

Ci sono due principali svantaggi per questo sistema. Quando il cambiamento è banale, non ha molto senso farlo rivedere. Tuttavia, dobbiamo assolutamente attenerci alle regole, per evitare la scivolosa inclinazione di dichiarare le modifiche "banali" quando non lo sono. D'altra parte, questo non è un ottimo modo per rivedere le modifiche significative al sistema o l'aggiunta di nuovi componenti di grandi dimensioni.

Abbiamo provato recensioni più formali prima, quando uno sviluppatore avrebbe inviato il codice email per essere esaminato al resto del team, quindi l'intero team si riunirebbe e discuterà. Questo ha richiesto molto tempo a tutti, e di conseguenza queste recensioni erano poche e lontane tra loro e solo una piccola percentuale della base di codice è stata rivista. L '"altra persona esamina le modifiche prima del commit" ha funzionato molto meglio per noi.

Mi piacciono le recensioni del codice, anche se possono essere dolorose. La ragione per cui mi piacciono è che ottengono più occhi sul codice e una prospettiva diversa. Credo che anche con la programmazione pair, il codice dovrebbe essere rivisto. È abbastanza facile per due persone che lavorano sullo stesso codice per fare collettivamente lo stesso errore che un diverso set di occhi non può mancare.

Se fatto come un gruppo con un proiettore, dovrebbe davvero essere rivisto individualmente prima della riunione. Altrimenti, è solo una fastidiosa perdita di tempo.

Ho fatto solo revisioni del codice via email e in gruppo. In generale, non penso che dovrebbero essere fatti di persona. Ti senti un po 'più di pressione per scorrere il codice con qualcuno che guarda oltre la tua spalla. Credo che uno strumento progettato per la revisione del codice sarebbe una buona risorsa, in quanto può aiutare con alcuni aspetti banali e dovrebbe rendere più facile contrassegnare i bit di codice del problema, quindi è via email.

Il problema di avere una sola persona fa tutte le revisioni del codice è che può essere un collo di bottiglia. Con standard di codifica ben documentati e progettati non dovrebbe essere necessario. A seconda dell'ambiente / programma di rilascio potrebbe essere una buona idea avere sempre qualcuno come revisore dei codici di attesa.

Credo che la proprietà del codice sia una buona idea in quanto questa persona può fare la sua priorità per capire quel codice e potenzialmente giocare un ruolo di gatekeeper.

In SmartBear non solo creiamo uno strumento di revisione del codice , lo usiamo anche giorno per giorno. È una parte essenziale del nostro processo di sviluppo. Esaminiamo ogni modifica che viene verificata.

Penso che sia una cattiva idea avere un unico revisore del codice gatekeeper per molte ragioni. Quella persona diventa un collo di bottiglia e devono fare troppa revisione del codice (solo per mantenere il progetto in movimento) per essere veramente efficace (60-90 minuti alla volta è il limite dell'efficacia). Le recensioni di codice sono un ottimo modo per condividere idee e conoscenze. Non importa quanto sia superstar il tuo gatekeeper, possono imparare dagli altri membri del team. Inoltre, avere tutti gli esami del codice crea anche un ambiente di "proprietà collettiva del codice", in cui le persone sentono di possedere la qualità del codice (non è solo il QA o il gatekeeper).

Abbiamo un whitepaper gratuito su " Best Practices for Peer Code Review "che ha 11 suggerimenti per rendere efficaci le revisioni del codice. Gran parte di questo è lo stesso contenuto del libro che John ha citato in una forma più distillata.

Nessuna scusa. Pratica programmazione della coppia. È la migliore revisione possibile del codice. Qualsiasi altro meccanismo si traduce in gioco di colpa. La programmazione di coppie induce spirito di squadra e proprietà collettiva. Inoltre, si discute le idee con la coppia, si fallisce velocemente, si intraprendono azioni correttive ed è solo la coppia codificata e sottoposta a revisione del codice che viene inserita in Configuration Management System (CMS). Felice coppia di programmazione!

Se stai lavorando a un progetto in cui più persone contribuiranno alla base di codice, è necessario stabilire uno standard.

A questo punto, nella mia esperienza è meglio designare una persona come il "re" della revisione del codice, se lo farai e quello che dice. A questo proposito, se un utente non aderisce agli standard, il re si prenderà cura di esso.

Come sviluppatore, rivedo molte volte il mio codice per essere leggibile, ragionevole e tutto il resto. Di solito utilizziamo javadoc o simili in determinate lingue con cui codifichiamo e usiamo il tag @author per associare la proprietà a funzioni, classi, ecc.

Se una funzione non è corretta, parliamo con il proprietario, lo stesso con la classe ecc.

Nella mia azienda a ogni attività viene assegnato un tester per testare l'attività e anche un revisore del codice per rivedere il codice.

Se il tuo prodotto è già stato rilasciato e vuoi essere certo di non fare nulla di sbagliato (come una perdita di handle o perdita di memoria) le recensioni di codice sono una grande cosa. Penso che durante lo sviluppo iniziale prima di rilasciare il tuo prodotto, le revisioni del codice potrebbero essere eccessive.

Se la tua squadra ha tutti gli sviluppatori senior, la revisione tra pari è comunque utile. Ognuno fa errori a volte. Se la tua squadra ha alcuni senior e alcuni junior, quindi lascia che gli sviluppatori senior facciano le revisioni del codice, ma mantengano comunque le revisioni del codice per il codice senior.

Una cosa importante della revisione del codice è che può rilevare gli errori che facciamo, ma non è un sostituto per i test.

Raccomando di utilizzare le revisioni del codice se non stai eseguendo la programmazione della coppia.

Non discutere i pro e i contro con l'abbinamento, ma è difficile contestare gli effetti positivi di avere il tuo codice costantemente rivisto da (almeno) un'altra persona. Il codice è anche scritto e progettato da (almeno) due programmatori - difficilmente può essere migliore di così. Sto dicendo "almeno" perché imo, dovresti provare a scambiare le coppie molto in modo che tutti possano provare a lavorare con il codice.

Una delle cose che cerco di fare nelle recensioni del codice a cui partecipo è dopo aver esaminato il codice da solo, eseguo analisi statiche del codice, usando strumenti come Findbugs, PMD, JavaNCCP e altri, e guardo a qualsiasi problema gli strumenti trovano all'interno del codice da rivedere. In particolare, desidero esaminare tutto ciò che presenta un livello insolitamente elevato di complessità, chiedendo loro di spiegare perché è necessario tale livello di complessità o perché la vulnerabilità suggerita non è importante.

YMMV

Dove lavoro attualmente produciamo dispositivi hardware e il software che li interfaccia con quelli che entrano nell'infrastruttura critica. Di conseguenza, ci concentriamo molto sulla qualità del rilascio. Utilizziamo una variante di Fagan Inspection e abbiamo un processo di revisione formale. Siamo certificati ISO 9001, tra le altre certificazioni.

Il settore delle infrastrutture critiche è molto interessato all'affidabilità e alla ripetibilità degli stessi. : -)

Nella mia azienda abbiamo revisioni obbligatorie del codice per i programmatori junior e revisioni volontarie per gli anziani. Non esiste un revisore del codice designato, le richieste di revisione vengono inviate a tutti gli sviluppatori.

Questo sistema funziona bene - le revisioni vengono eseguite quando il tempo lo permette e le modifiche possono essere riviste da diversi gruppi di bulbi oculari.

Lo strumento eccellente e gratuito Review Board funziona molto bene per noi e si è dimostrato un ottimo modo per comunicare recensioni.

Nella mia azienda, non eseguiamo mai revisioni formali del codice prima dei controlli, a meno che non stiamo modificando alcune produzioni altamente critiche e non abbiamo il tempo di eseguire il nostro processo QA standard.

Ciò che facciamo è che ogni volta che sentiamo che sarebbe utile una revisione del codice, aggiungiamo un commento "// todo: riesame del codice by joe" al codice modificato. Di solito, selezioniamo Joe perché possiede il codice modificato, ma se questo criterio di selezione non si applica (di solito lo fa), abbiamo scelto qualcun altro a caso. E, naturalmente, se Joe sembra essere disponibile in quel momento, usiamo il buon vecchio metodo di revisione sopra la spalla.

Come revisore, l'unica cosa che devi fare è cercare periodicamente l'intero codice per "// todo: revisione del codice da parte mia" , rivedere le modifiche e ricontrollare il codice senza il "todo ..." commento

Se c'è un problema, torniamo ai metodi di comunicazione tradizionali (mail / chat / etc).

Questo metodo ci fornisce le due qualità principali che stiamo cercando in un sistema di recensioni:

• overhead molto leggero
• tracciabilità

Troviamo il modo più efficace per fare le revisioni del codice è 1-su-1 usando github per mostrare le differenze nel ramo.

• Una persona è considerata il custode della qualità e rivede il codice o il team possiede lo standard?

  • Dipende dalle dimensioni del team: un team di 3 persone avrà 1 senior con la migliore conoscenza delle buone pratiche, mentre un team di 12 potrebbe avere 3 o 4 persone che svolgeranno tale ruolo.

• Riesci a riesaminare il codice come esercizio di gruppo utilizzando un proiettore?

  • Di persona. 1 su 1 per essere meno minaccioso. A volte fatto nell'area comunale anche se per diffusione di conoscenza. Dipende dalla situazione esatta, persone, orari, ecc.

• È fatto di persona, via email o usando uno strumento?

  • Di persona. Usiamo git e github e ha un'ottima revisione del codice e strumenti di confronto diff per semplificare la revisione del codice.

• Ti rifuggono le recensioni e usi elementi come la programmazione delle coppie e la proprietà del codice collettivo per garantire la qualità del codice?

  • Proviamo a fare entrambi i casi in modo appropriato. Ciò significa che quando si è bloccati su un problema particolarmente spinoso, o quando si lavora sull'infrastruttura principale o quando si sta preparando per le vacanze o quando si lascia l'azienda, si può fare il pairing per condividere e trasferire conoscenze. La maggior parte del codice o codice che ha qualcosa di più di modifiche estetiche dovrebbe essere riesaminata alla fine.

Come per tutti gli elementi di codifica, la risposta giusta dovrebbe tener conto:

• Tipo di azienda (ad es. startup o grande azienda)
• Dimensioni dell'azienda
• Numero di sviluppatori
• Budget
• Calendario
• Carico di lavoro
• Complessità del codice
• Capacità dei revisori
• Disponibilità dei revisori

Ho lavorato in molte aziende e ho visto molti processi. Il mio team attuale gestisce ciò che di meglio ho visto finora.

Usiamo un processo di sviluppo agile e come parte di esso abbiamo le porte che ogni storia deve attraversare. Una di queste porte è la revisione del codice. La storia non si sposta per testare fino a quando la revisione del codice non è stata completata.

Inoltre, memorizziamo il nostro codice su github.com. Quindi, dopo che uno sviluppatore ha completato la sua modifica, pubblica il link ai commit sulla storia.

Quindi taggliamo un altro sviluppatore da recensire. Github ha un sistema di commenti che consente a qualcuno di commentare direttamente sulla riga di codice in questione. Github invia quindi un'email alla distro, quindi a volte otteniamo occhi extra da alcuni dei nostri altri team.

Questo processo ha funzionato molto bene per noi. Utilizziamo uno strumento di processo agile che semplifica la pubblicazione dei commit e facilita la comunicazione.

Se un problema è particolarmente appiccicoso, ci sediamo e discutiamo. Questo funziona perché è parte integrante del nostro processo e tutti hanno acquistato il modo in cui funziona il processo. Possiamo spostare i biglietti di nuovo in corso se la revisione del codice risulta nella rielaborazione necessaria e quindi può essere riesaminata dopo che sono state apportate modifiche, o il revisore potrebbe notare sul fatto che la correzione degli articoli è sufficiente e non necessita di una revisione.

Se il test riprende qualcosa, torna in corso e vengono esaminate anche eventuali altre modifiche.