Qual è il modo migliore per un programmatore di gestire la revisione del codice?

È vero, trovare la difensiva non è d'aiuto; ma poi - nessuno dei due viene criticato, quindi se pensi che stia succedendo dovresti davvero vocalizzare le tue preoccupazioni sul fatto che la revisione del codice non ti aiuta a scrivere codice migliore all'interno dell'organizzazione.

Il revisore ha la responsabilità di rivedere il codice per vere e proprie non conformità e difetti, non usarlo come mezzo per scrivere il codice nel modo in cui avrebbero fatto. Ciò significa che la recensione è lì per rivedere come hai fatto qualcosa, e indicare tutte le aree che hai commesso un errore (qualcosa che il meglio di noi fa) o non ho capito il quadro o gli standard o "ragioni storiche" è stato scritto un codice così è dove ti trovi.

Quindi, se ci sono diversi modi di fare qualcosa, ci deve essere una buona ragione per cui il tuo codice è cambiato in un modo alternativo, altrimenti è semplicemente una zangola non costruttiva che non ti aiuterà.

Inoltre, ricorda che anche il recensore non è perfetto. Potrebbero avere l'idea che Y sia il modo per farlo e non aver capito che X è migliore. Dovresti spiegare i motivi per cui l'hai fatto in modo X. Il revisore potrebbe essere d'accordo con te, o potrebbe dirti perché Y è una soluzione migliore - potrebbero esserci altri fattori che non sai che lui fa.

In breve, le recensioni sono un modo per far comunicare ai membri del team le modifiche al codice. Quindi parla con il revisore.

SE aiuta, parla in modo imparziale, come se non fossi nemmeno l'autore del codice in fase di revisione. Il codice appartiene alla società o al team, dopotutto, e il team dovrà mantenerlo. Sei semplicemente il tipo che l'ha scritto, non è un fattore tanto importante come molti credono.

Scrivere il codice del computer è un ottimo esempio di prendere decisioni in condizioni di incertezza . Ci sono sempre pressioni contrastanti e il modo in cui decidi in una determinata domanda dipende da quali pressioni percepisci e da quanto le consideri grande.

Pertanto, quando un revisore non è d'accordo con la tua decisione, significa che vedono diverse pressioni / rischi di te, o che considerano alcuni di loro più grandi o più piccoli di te. Devi assolutamente parlare di queste differenze, perché non farlo perpetua i problemi che hanno portato al disaccordo in primo luogo.

Se il tuo revisore è più anziano, la loro esperienza potrebbe dire loro correttamente che questo o quel rischio non è molto rilevante nella pratica, o potrebbero sapere che un qualche tipo di errore ha una lunga storia di accadimento nella tua organizzazione, e vale la pena stai molto attento a evitarlo. Viceversa, se ritieni di sapere qualcosa che probabilmente il tuo revisore non lo fa, devi assolutamente esprimerlo; tacere equivale a un abbandono del dovere da parte vostra.

La parte più importante della gestione della situazione è capire che la critica alle decisioni di progettazione è praticamente sempre non una critica alla personalità di qualcuno. (Nei rari casi in cui si trova effettivamente, lo noterai abbastanza presto, e se davvero non riesci a compiacere qualcuno, niente di ciò che fai fa differenza, quindi dove è il danno nel seguire le migliori pratiche? Molto meglio trovare una posizione migliore come il più presto possibile). È solo il risultato di diverse persone che hanno percezioni diverse dei numerosi rischi e benefici implicati nel codice del computer, e dato il complesso codice del computer moderno, è solo da aspettarselo. Parlare delle differenze aiuta a migliorare il codice e migliorando la tua cooperazione in questo caso e in casi futuri.

Le altre risposte contengono già ottime informazioni. Vorrei dilungarmi un po 'su alcuni aspetti che sono stati suggeriti da gbjbaanb (vedi il mio commento alla sua risposta).

Nella mia esperienza ho osservato diversi tipi di feedback durante le revisioni del codice:

1. "Onestamente credo che sia sbagliato / non ottimale e che dovresti cambiarlo in questo modo." Di solito prendo seriamente questo tipo di feedback, e mi opporrò solo al cambiamento suggerito se penso di avere un punto di forza contro di esso.
2. "Trovo che la soluzione sia OK, considera questa alternativa, ma dipende da te se accetti il cambiamento o meno." Prendo questo tipo di feedback anche sul serio: il revisore suggerisce un'alternativa, anche se non hanno una strong opinione che la loro soluzione sia migliore. Ho l'opportunità di imparare qualcosa e accetterò il cambiamento se mi piacerà di più. Altrimenti, il revisore lo trova OK se tengo il codice secondo i miei gusti.
3. "Avrei fatto diversamente, quindi devi cambiarlo, punto.", o anche "Oh, ho cambiato il tuo codice perché ...", il cambiamento non è stato suggerito, è già stato commesso! Viene fornita una breve spiegazione sul cambiamento, con il suggerimento che non c'è molto tempo per discutere i dettagli perché dobbiamo passare all'attività successiva.
4. Il revisore suggerisce piccoli cambiamenti di natura banale che non migliorano il codice ma lo rendono diverso. Quando viene chiesto di discutere delle modifiche suggerite, il revisore entra in lunghe discussioni su dettagli insignificanti fino a quando non si arrende per esaurimento.

Le opzioni 3, 4 potrebbero venire mascherate come 1 e 2: "Era davvero importante farlo nel modo che ho suggerito." oppure "Puoi rifiutare il cambiamento se lo desideri veramente", ha detto con un tono che in realtà significa esattamente l'opposto di ciò che viene detto. Se ti opponi a modifiche che consideri non necessarie, la proprietà del codice condivisa può essere utilizzata come giustificazione di questo atteggiamento: "Il codice non appartiene a te, appartiene al team!" Si può dire quando l'intenzione del revisore non è onesta se il revisore non è molto aperto alla discussione, si irrita e cerca di spingere la propria soluzione a tutti i costi. Fondamentalmente hanno già deciso, e ogni ulteriore discussione è solo una perdita di tempo.

Le opzioni IMO 1 e 2 sono un segno di un revisore onesto che sta cercando di aiutare, sta cercando di insegnarti qualcosa nel rispetto del tuo lavoro, ed è anche aperto a imparare qualcosa da solo. In questo scenario cerco di non essere troppo orgoglioso quando ottengo feedback costruttivi da un revisore.

Le opzioni 3, 4 suggeriscono che c'è qualche gioco di potere in corso: l'importante è se lo facciamo a modo mio o a modo tuo, non che cerchiamo di trovare una buona soluzione che soddisfi entrambi. Questo può essere correlato all'ego del recensore, ma anche a loro non essere in grado di comprendere alcun codice che non segua il loro modo di pensare. Normalmente sono disturbati da un codice che non sembra familiare a loro e vorrebbe imporre la loro strada sull'intero codice base.

Se le situazioni 3 e 4 si verificano troppo spesso, il lavoro di squadra può diventare piuttosto spiacevole. In una situazione del genere, prenderei in considerazione l'idea di lasciare la squadra.

Affrontare il tuo problema su cosa fare quando non sei d'accordo ...

Parlarne è la strada da fare, come hanno già sottolineato molte persone.

Se lo hai già fatto, forse più di una volta, una tecnica utile che usiamo è che se c'è ancora disaccordo in alcune aree è dire 'sì, sarebbe un buon refactoring che -

Possiamo inserire un biglietto separato per questo?

Un biglietto separato ti consente di ottenere il codice, invece della temuta modalità "abbandona e non muovi mai" che ho conosciuto bene in alcuni posti. Ciò si adatta bene all'agile, facendo la minima quantità possibile e diffondendo il carico. A volte yagni finirà per fare domanda. Alcune volte il product manager deciderà che ci sono esigenze più urgenti di quel refactoring in termini di valore per il cliente, scadenze e risorse.

La revisione del codice è probabilmente una buona cosa in generale, ma dalla mia esperienza è servito come strumento per gli sviluppatori di nuovi team che utilizzano nuove linee guida di codifica o per correggere bug importanti, in modo da ridurre il rischio di errori. Se ritieni di conoscere meglio il revisore, dovresti chiedere perché la soluzione che lui o lei propone è migliore e discutere con le tue opinioni in merito. Nessuno sa tutto di meglio, quindi la revisione del codice dovrebbe o potrebbe essere una preziosa esperienza di apprendimento per entrambi.

La revisione del codice è sia un'opportunità per cogliere potenziali problemi e trasmettere conoscenze, sia per i revisori che per i programmatori.

Come revisore del codice, la responsabilità è di evidenziare possibili aree di rischio, non conformità alle prassi standard, miglioramenti e, in generale, solo un altro punto di vista sulla stessa area di codice.

Questo non dovrebbe comportare una modifica al codice senza discutere / comprendere le decisioni dei programmatori al momento dello sviluppo.

Se il revisore sta apportando modifiche, potrebbe avere difficoltà a delegare il lavoro agli altri, il che è difficile da fare per molte persone intelligenti.

Come programmatore che riceve una recensione, sei protetto da un possibile problema quando viene distribuito, hai la possibilità di imparare qualcosa di nuovo e l'opportunità di condividere le tue conoscenze con il revisore.

Indipendentemente dall'anzianità, il tuo modo di pensare può venire con una soluzione che non si presenta ad alcuni, quindi la recensione può essere la tua occasione per brillare solo facendo ciò che ritieni sia giusto.

Fintanto che sia il programmatore che il revisore accettano che possono essere sbagliati e che è giusto che sia sbagliato, una recensione diventa qualcosa che rafforza il team perché si lavora insieme sulla soluzione.

Sembra che il tuo codice non sia ancora stato revisionato: -)

L'obiettivo della revisione del codice è ottenere il codice in una qualità decente e sapere che hai un codice di buona qualità. Quando viene riesaminato il codice di uno sviluppatore inesperto, può essere utilizzato per insegnare come scrivere codice migliore, evitando allo stesso tempo di frustrare lo sviluppatore.

Il revisore dovrebbe mai cambiare il tuo codice. Possono fare suggerimenti più o meno forti su come vorrebbero che il tuo codice venga modificato e possono decidere se accettare o meno il tuo codice.

Se la recensione va bene / se rivedo il tuo codice, quello che probabilmente otterrai sono alcuni commenti su come I scriverebbe il codice da cui puoi imparare, o ignorare - queste sono cose in cui io avere un'opinione e sei libero di avere un'opinione diversa. Nella mia area, una buona denominazione di funzioni, variabili e così via è considerata importante, quindi potresti ricevere alcuni suggerimenti per migliorare la denominazione. Di solito dovresti apportare delle modifiche in quel caso (a volte trovando un nome ancora migliore per qualcosa). A volte trovo bug. Li risolvi. A volte trovo cose che io sono bug e mi sbaglio. Se è difficile vedere che il codice è corretto, lo rendi più ovvio corretto. Se ho appena sbagliato, dimmelo tu.

Se penso che il design non sia generalmente corretto, allora questo avrebbe dovuto essere discusso in precedenza. Dovremmo quindi pensare se il tuo progetto è abbastanza buono, tenendo conto di quanto lavoro è coinvolto in un cambiamento, o forse ho sbagliato e il tuo design è migliore. Dovremmo finire con un accordo.

Se il revisore e il recensore non sono d'accordo, allora abbiamo un problema. Perché significa che uno di noi è incapace di lavorare in gruppo, o uno di noi è incapace di distinguere tra un progetto buono o cattivo o entrambi. Questa non è necessariamente colpa tua. Sfortunatamente ci sono sviluppatori che sono senior e clueless, e questo sarà un problema per l'azienda e per te.

Se succede, pensa molto, molto duramente: hai problemi ad accettare critiche ben fondate? Se è così, devi cambiare atteggiamento. Sei troppo inesperto per capire perché il recensore ha ragione? Se è così, non c'è problema. Affidati al revisore e impara. Sei sicuro di conoscere meglio del recensore? Accetta la recensione, ma chiedi a un terzo sviluppatore fidato circa la sua opinione. Ricorda che puoi essere veramente sicuro di te stesso e avere ragione, ma puoi anche essere veramente sicuro di te stesso e sbagliare.