Buone linee guida e pratiche per la revisione obbligatoria del codice [chiusa]

11

Stiamo provando la revisione obbligatoria del codice su ogni commit: non c'è nulla che arrivi al master che non sia stato convalidato da almeno 1 persona e non dall'autore - per un paio di sprint. Abbiamo acquistato da entrambi gli sviluppatori e il management (che è una situazione eccezionale) e vogliamo ottenere alcuni dei vantaggi per cui è noto:

  • ovvia riduzione dei bug
  • una maggiore consapevolezza dei cambiamenti che si verificano attorno al progetto
  • il "so che qualcuno lo guarderà quindi non sarò pigro" / effetto anti-cowboy
  • maggiore coerenza all'interno / tra progetti

Ma stiamo introducendo qualcosa che è conosciuto per ridurre la velocità, e se fatto male potrebbe creare uno stupido passaggio burocratico nella pipeline del commit che non fa altro che occupare del tempo. Cose di cui sono preoccupato:

  • recensioni relative alla selezione nitida
  • (iperbolicamente) persone che aprono enormi problemi architettonici come parte di una revisione del commit in due righe.
  • Non voglio influenzare le risposte con altre cose.

Mentre siamo tutti persone ragionevoli e faremo molte analisi di sé, potremmo sicuramente utilizzare alcune informazioni acquisite in battaglia su quali tipi di cose dovremmo cercare di ottenere in una sessione di revisione per fare davvero recensioni lavora per noi. Quali sono alcune linee guida e criteri che hai trovato funzionanti?

    
posta quodlibetor 09.12.2013 - 19:47
fonte

3 risposte

13
  1. Scrivi recensioni brevi.

    È difficile rimanere concentrati, specialmente durante la revisione del codice, per molto tempo. Inoltre, le revisioni lunghe del codice possono indicare che c'è troppo da dire sul codice (vedi i prossimi due punti) o che la recensione diventa una discussione su problemi più grandi, come l'architettura.

    Inoltre, una recensione dovrebbe rimanere una recensione, non una discussione. Ciò non significa che l'autore del codice non possa rispondere, ma non dovrebbe trasformarsi in un lungo scambio di opinioni.

  2. Evita di rivedere il codice che è troppo brutto.

    La revisione del codice di bassa qualità è deprimente sia per il revisore che per l'autore del codice. Se un pezzo di codice è terribile, una revisione del codice non è utile. Invece, all'autore dovrebbe essere chiesto di riscrivere correttamente il codice.

  3. Utilizza i controllori automatici prima della revisione.

    I controllori automatici evitano di sprecare tempo prezioso a individuare errori che possono essere trovati automaticamente. Ad esempio, per il codice C #, l'esecuzione di StyleCop, le metriche del codice e in particolare l'analisi del codice rappresentano una buona opportunità per trovare alcuni errori prima della revisione. Quindi, la revisione del codice può essere speso per punti che sono estremamente difficili da fare per una macchina.

  4. Scegli attentamente le persone che fanno recensioni.

    Due persone che non possono sopportare l'un l'altro non faranno una buona recensione del codice di un'altra persona. Lo stesso problema si presenta quando una persona non rispetta l'altra (non importa se sia il revisore o l'autore, comunque).

    Inoltre, alcune persone non sono in grado di vedere il loro codice rivisto, quindi richiedono una formazione specifica e una preparazione per capire che non sono criticate e che non dovrebbero vederlo come qualcosa di negativo. Fare una recensione, impreparato, non aiuta, dal momento che sarà sempre sulla difensiva e non ascolterà alcun critico del proprio codice (prendendo ogni suggerimento come critica).

  5. Esegui revisioni sia informali che formali.

    Avere una lista di controllo aiuta a concentrarsi su una serie precisa di difetti, evitando di dedicarsi alla raccolta nitida. Questo elenco di controllo può contenere punti come:

    • SQL Injection,
    • Assunzioni errate su un linguaggio che può portare a errori,
    • Situazioni specifiche che possono portare a errori, come la precedenza degli operatori. Ad esempio, in C #, var a = b ?? 0 + c ?? 0; potrebbe sembrare soddisfacente per qualcuno che desidera aggiungere due numeri nullable con coalesce a zero, ma non lo è.
    • Deallocation of memory,
    • Caricamento lento (con i suoi due rischi: caricare la stessa cosa più volte e non caricarlo affatto),
    • overflow,
    • Strutture dati (ad esempio errori come una semplice lista invece di un set di hash),
    • Convalida dell'input e programmazione difensiva in generale,
    • Sicurezza dei thread,
    • ecc.

    Interrompo la lista qui, ma ci sono centinaia di punti che possono figurare in una checklist, a seconda dei punti deboli di un autore preciso.

  6. Regola progressivamente la lista di controllo.

    Per rimanere costruttivi e utili nel tempo, le liste di controllo utilizzate nelle revisioni formali dovrebbero essere modificate nel tempo a seconda degli errori riscontrati. Ad esempio, le prime recensioni informali possono rivelare una certa quantità di rischi di SQL Injection. Il controllo dell'iniezione SQL sarà incluso nell'elenco di controllo. Quando, pochi mesi dopo, sembra che l'autore sia ora molto attento alle query dinamiche e parametrizzate, SQL Injection potrebbe essere rimosso dalla checklist.

risposta data 09.12.2013 - 20:15
fonte
2

Abbiamo quasi un elenco di controllo:

  • Mostrami la descrizione dell'attività.
  • Fammi vedere il risultato e mostralo lavoro. Esegui scenari diversi (input non validi, ecc.).
  • Fammi vedere il test di passaggio. Come è la copertura del test?
  • Fammi vedere il codice - è qui che stiamo cercando evidenti inefficienze.

Funziona abbastanza bene.

    
risposta data 09.12.2013 - 21:33
fonte
0

Penso che una persona che ha potere sugli altri sarebbe sufficiente, amministratore o moderatore per tagliare commenti irrilevanti, accelerare la revisione di cose che richiedono una rapida revisione. Unico decisore.

Meno di questo sarebbe che questa persona deve farlo come compito principale, mentre potrebbe fare qualcos'altro, e probabilmente ti piacerebbe avere la persona più esperta in questa posizione.

La seconda cosa è automatizzare il più possibile!

  • controllo degli spazi bianchi
  • software di controllo dello stile
  • build automatizzati prima della revisione del codice
  • test automatizzati prima della revisione del codice

Quelle cose rimuoveranno almeno alcune cose che le persone potrebbero commentare senza reali necessità. Se non sta costruendo o ha spazi vuoti finali non è sufficiente per la revisione, risolvilo e richiedi nuovamente la revisione. Se non sta costruendo o alcuni test falliscono, è ovvio che non è abbastanza buono.

Molto dipende dalle tue tecnologie, ma trova ciò che puoi controllare automaticamente più è meglio.

Non abbiamo ancora vinto questa battaglia, ma è quello che abbiamo trovato utile.

    
risposta data 09.12.2013 - 20:18
fonte

Leggi altre domande sui tag