Come gestisco il disaccordo in una revisione del codice riguardante un caso limite improbabile?

an obscure case that is extremely unlikely to happen--in fact I'm not sure it is even possible to occur

Non avere comportamenti non testati nel codice può essere molto importante. Se viene eseguito un codice, ad es. 50 volte al secondo, una possibilità su un milione si verifica all'incirca ogni 5,5 ore di autonomia. (Nel tuo caso, le probabilità sembrano inferiori.)

Puoi parlare delle priorità con il tuo manager (o chiunque sia una persona più anziana responsabile per l'unità in cui lavori). Capirai meglio se ad es. lavorare sulle prestazioni del codice o sul codice a prova di proiettile è la massima priorità e quanto improbabile possa essere il caso d'angolo. Il tuo revisore potrebbe anche avere un'idea distorta delle priorità. Avendo parlato con la persona responsabile, avrai un tempo più facile (dis) concordare con i suggerimenti del tuo revisore e avremo qualcosa a cui fare riferimento.

È sempre una buona idea avere più di un revisore. Se il tuo codice viene esaminato solo da un collega, chiedi a qualcun altro che conosce quel codice o il codice base in generale, di dare un'occhiata. Un secondo parere, ancora una volta, ti aiuterà a semplificare (dis) l'accordo con i suggerimenti del recensore.

Avere un numero di commenti ricorrenti durante diverse revisioni di codice di solito indica che una cosa più grande non viene comunicata chiaramente, e gli stessi problemi emergono ancora e ancora. Cerca di scoprire quella cosa più grande e discuterla direttamente con il revisore. Chiedi abbastanza perché domande. Mi ha aiutato molto quando ho iniziato la pratica delle revisioni del codice.

Posso darti un esempio di caso d'angolo che non potrebbe mai accadere che ha causato un disastro.

Quando è stato sviluppato il Ariane 4 i valori dal laterale accelerometri sono stati ridimensionati per rientrare in un numero intero con segno a 16 bit e poiché la massima potenza possibile degli accelerometri, se ridimensionata, potrebbe mai superare i 32767 e il minimo potrebbe mai caduta sotto -32768 non c'era "non è necessario per il sovraccarico del controllo di gamma". In generale, tutti gli input dovrebbero essere controllati dal range prima di qualsiasi conversione, ma in questo caso si cercherebbe di catturare un caso d'angolo impossibile.

Diversi anni dopo è stato sviluppato il Ariane 5 e il codice per ridimensionare gli accelerometri laterali è stato riutilizzato con test minimi come era "provato in uso". Sfortunatamente il razzo più grande poteva aspettarsi maggiori accelerazioni laterali, quindi gli accelerometri sono stati aggiornati e potrebbero produrre più grandi 64-bit float valori.

Questi valori più grandi sono "racchiusi" nel codice di conversione, ricorda no controllo intervallo, e i risultati del primo lancio nel 1996 non sono stati buoni. Costa ai milioni di aziende e ha causato una grande interruzione nel programma.

Ilpuntochestocercandodifareèchenondovrestiignorareicasiditestcomemai,estremamenteimprobabile,ecc.:glistandardchestavanocodificandoperhachiestoilcontrollodell'intervallodituttivaloridiinputesterni.Sequestofossestatotestatoegestito,alloraildisastroavrebbepotutoessereevitato.

NotacheinAriane4questononeraunbug,(datochetuttofunzionavabeneperognipossibilevalore)-eraunerroreseguireglistandard.Quandoilcodiceèstatoriutilizzatoinunoscenariodiverso,hafallitoinmodocatastrofico,mentresel'intervallodivalorifossestatoritagliatosarebbeprobabilmentefallitocongarbo,el'esistenzadiuncasoditestperquestopotrebbehannoattivatounarevisionedeivalori.Valeanchelapenanotareche,mentreiprogrammatorieitestersonointervenutiperalcunecritichedagliinvestigatorichehannoseguitol'esplosione,ladirezione,ilcontrolloqualitàel'assistenzalaleadershipèstatalasciataconlamaggiorpartedellacolpa.

Chiarimento

Anchesenontuttoilsoftwareècriticoperlasicurezza,nécosìspettacolarequandofallisce,lamiaintenzioneeraquelladievidenziarecheitest"impossibili" possono ancora avere valore. Questo è il caso più drammatico che io conosca, ma la robotica può anche produrre alcuni risultati disastrosi.

Personalmente, direi che una volta che qualcuno ha evidenziato un caso limite al team di test, dovrebbe essere messo a punto un test per controllarlo. Il responsabile del team di implementazione o il responsabile del progetto possono decidere di non tentare di risolvere eventuali errori rilevati, ma devono essere consapevoli del fatto che esistono carenze. In alternativa, se il test è troppo complesso, o costoso, da implementare, un problema può essere sollevato in qualunque tracker è in uso e / o il registro dei rischi per chiarire che questo è un caso non verificato - che potrebbe dover essere affrontato prima di un cambio di utilizzo o impedire un uso inappropriato.

Poiché non è stato gestito in precedenza, è fuori dall'ambito del tuo impegno. Tu o il tuo collega potete chiedere al vostro manager se vale la pena di coprire questo caso.

Con algoritmi complessi, è molto difficile dimostrare di aver pensato a ogni caso di test che verrà fuori nel mondo reale. Quando lasci intenzionalmente un caso di test interrotto perché non verrà visualizzato nel mondo reale, potresti potenzialmente lasciare altri casi di test interrotti a cui non hai ancora pensato.

L'altro effetto che accade spesso è quando gestisci ulteriori casi di test, l'algoritmo diventa necessariamente più generale e, a causa di ciò, trovi dei modi per semplificarlo e renderlo più solido per tutti il tuo test casi. Ciò consente di risparmiare tempo durante la manutenzione e la risoluzione dei problemi lungo il percorso.

Inoltre, ci sono mille casi in cui "questo non dovrebbe mai accadere" "bug che si verificano in natura. Questo perché il tuo algoritmo potrebbe non cambiare, ma i suoi input potrebbero cambiare anni lungo la strada quando nessuno si ricorda di questo caso d'uso non gestito. Ecco perché gli sviluppatori esperti gestiscono questo tipo di cose quando sono fresche nella loro mente. Ritorna a morderti più tardi se non lo fai.

Questa non è una domanda tecnica ma una decisione di strategia aziendale. Si nota che la soluzione suggerita è per un caso molto oscuro che non accadrà mai quasi . Qui fa una grande differenza se stai programmando un giocattolo o se stai programmando dire attrezzature mediche o un drone armato. Le conseguenze di un malfunzionamento raro saranno molto diverse.

Quando si eseguono revisioni di codice, è necessario applicare una conoscenza di base delle priorità aziendali al momento di decidere quanto investire nella gestione di casi rari. Se non sei d'accordo con il tuo collega nell'interpretazione delle priorità dell'azienda, potresti voler discutere con qualcuno sul lato economico delle cose.

Le revisioni del codice non riguardano esclusivamente la correttezza del codice. In realtà, questo è piuttosto in basso nella lista dei benefici, dietro la condivisione delle conoscenze e un processo lento ma costante verso la creazione di un consenso di stile / design di squadra.

Come stai vivendo, ciò che conta come "corretto" è spesso discutibile, e ognuno ha il proprio punto di vista su ciò che è. Nella mia esperienza, il tempo e l'attenzione limitati possono anche rendere le revisioni del codice molto incoerenti: lo stesso problema potrebbe essere rilevato o meno a seconda di sviluppatori diversi e in momenti diversi dallo stesso sviluppatore. I revisori nella mentalità di "cosa migliorerebbe questo codice?" suggerirà spesso cambiamenti che non aggiungerebbero ai loro sforzi in modo naturale.

Come programmatore esperto (più di 15 anni come sviluppatore), sono spesso recensito da programmatori con meno anni di esperienza di me. A volte mi chiedono cambiamenti che non sono d'accordo o che non mi sembrano importanti. Tuttavia, continuo a fare quei cambiamenti. Combatto le battaglie sui cambiamenti solo quando ritengo che il risultato finale provocherebbe una debolezza del prodotto, dove il costo del tempo è molto alto, o dove un punto di vista "corretto" può essere reso oggettivo (ad es. t lavoriamo nella lingua che stiamo usando, o un benchmark mostra che un miglioramento delle prestazioni richiesto non è uno).

Quindi ti suggerisco di scegliere attentamente le tue battaglie. Due giorni di codifica di un caso di test che ritieni non sia necessario probabilmente non vale il tempo / lo sforzo per combattere. Se stai usando uno strumento di revisione, come le richieste di pull di GitHub, allora forse commenta il costo / beneficio che percepisci, al fine di notare la tua obiezione, ma accetta di continuare a lavorare. Questo conta come un delicato respingimento, quindi il revisore sa che stanno raggiungendo un confine e, cosa più importante, include la logica in modo che casi come questo possano essere intensificati se entrano in una situazione di stallo. Desiderate evitare l'escalation dei disaccordi scritti - non volete avere un argomento in stile forum su Internet per le differenze di lavoro - quindi potrebbe essere utile parlare prima di tutto del problema e registrare un riepilogo corretto del risultato della discussione, anche se voi accetti ancora di non essere d'accordo sulla necessità del lavoro (è del tutto possibile che una breve discussione amichevole deciderà comunque per entrambi).

Dopo l'evento, questo è un buon argomento per le riunioni di pianificazione sprint o di pianificazione del team di sviluppo, ecc. Presentalo in modo neutrale, ad es. "Nella revisione del codice, lo sviluppatore A ha identificato questo caso di test aggiuntivo, ci sono voluti altri due giorni per scrivere, la squadra pensa che la copertura extra sia giustificata a questo costo?" - questo approccio funziona molto meglio se fai effettivamente il lavoro, in quanto ti mostra in una luce positiva; hai svolto il lavoro, e vuoi solo sondare la squadra per l'avversione al rischio e lo sviluppo delle funzionalità.

Ti consiglierei di affermare almeno contro il caso oscuro. In questo modo, non solo i futuri sviluppatori vedono che hai deciso attivamente contro il caso, ma con una buona gestione degli errori, che dovrebbe già essere in atto, questo sorprenderebbe anche.

E poi, fai un test che afferma che il fallimento. In questo modo, il comportamento è meglio documentato e verrà mostrato nei test unitari.

Questa risposta ovviamente presuppone che il tuo giudizio sull'essere persino "estremamente improbabile, se possibile" sia corretto e non possiamo giudicarlo. Ma se lo è, e il tuo collega è d'accordo, allora un'esplicita affermazione contro l'evento dovrebbe essere una soluzione soddisfacente per entrambi.

Poiché sembra che tu sia nuovo lì, c'è solo una cosa che puoi fare: controlla con il capo squadra (o il capo progetto). 13 ore sono una decisione aziendale; per alcune ditte / squadre, molto; per alcuni, niente. Non è una tua decisione, non ancora.

Se il lead dice "cover that case", bene; se dice "nah, fanculo", bene - la sua decisione, la sua responsabilità.

Per quanto riguarda le revisioni del codice in generale, rilassati. Avere un'attività restituita una volta o due è perfettamente normale.

Una cosa che non penso di aver visto indirizzato in natura, sebbene fosse un po 'sollevato nella risposta di @SteveBarnes:

Quali sono le ripercussioni di un errore?

In alcuni campi un errore è un errore su una pagina web. Un PC blu scherma e si riavvia.

In altri campi è la vita o la morte: l'auto che guida si blocca. Il pacificatore medico smette di funzionare. O nella risposta di Steve: roba esplosa con conseguente perdita di milioni di dollari.

C'è un mondo di differenza in quegli estremi.

Indipendentemente dal fatto che 13 ore per coprire un "fallimento" valga la pena, in ultima analisi, non dovrebbe dipendere da te. Dovrebbe spettare alla direzione e ai proprietari. Dovrebbero avere un'idea di un'immagine più grande.

Dovresti essere in grado di dare una buona idea di quello che ne varrà la pena. Il tuo robot semplicemente rallenterà o si fermerà? Prestazioni degradate? O un fallimento del robot causerà un danno monetario? Perdita della vita?

La risposta a QUESTA domanda dovrebbe portare la risposta a "vale 13 ore del tempo delle aziende". Avviso: ho detto al tempo delle società. Pagano le bollette e alla fine decidono cosa ne vale la pena. La tua gestione dovrebbe avere l'ultima parola in ogni caso.

Forse parlare con la persona responsabile della priorità del lavoro? In avvio potrebbe essere CTO o proprietario del prodotto. Potrebbe aiutare a scoprire se questo lavoro extra è richiesto e perché. Inoltre puoi portare le tue preoccupazioni durante gli standup quotidiani (se li hai).

Se non c'è una chiara responsabilità (ad esempio il proprietario del prodotto) per il lavoro di planata, prova a parlare con le persone intorno a te. Più tardi potrebbe diventare un problema che tutti stanno spingendo il prodotto in direzione opposta.

Il modo migliore per gestire il disaccordo è lo stesso, indipendentemente dal fatto che tu sia uno sviluppatore junior o uno sviluppatore senior o addirittura un amministratore delegato.

Agisci come Columbo .

Se non hai mai visto nessun Columbo, è stato uno spettacolo piuttosto fantastico. Columbo era un personaggio molto modesto - la maggior parte della gente pensava che fosse un po 'pazzo e non valesse la pena di preoccuparsi. Ma apparendo umile e chiedendo alla gente di spiegare è riuscito a ottenere il suo uomo.

Penso che sia anche correlato al metodo socratico .

In generale, vuoi porre domande a te stesso e agli altri per assicurarti di fare le scelte giuste. Non da una posizione di "Ho ragione, ti sbagli", ma da una posizione di scoperta onesta. O almeno il meglio che puoi.

Nel tuo caso hai due idee qui, ma hanno fondamentalmente lo stesso obiettivo: rendere il codice migliore.

Hai l'impressione che il modo migliore per farlo sia di evitare la copertura del codice per un caso potenzialmente improbabile (impossibile?) a favore dello sviluppo di altre funzionalità.

Il tuo collaboratore ha l'impressione che essere più attento ai casi d'angolo sia più prezioso.

Cosa vedono che non vedi? Cosa vedi che non vedono? Essendo uno sviluppatore junior, in realtà sei in un'ottima posizione perché naturalmente dovrebbe porre domande. In un'altra risposta qualcuno menziona quanto sorprendentemente probabile sia un caso d'angolo. Quindi puoi iniziare con "Aiutami a capire - Ho avuto l'impressione che X, Y e Z - cosa mi manca? Perché il frambobozle del widget?" Avevo l'impressione che sarebbe fintubio in circostanze cromatiche. lo swizzle stick in realtà embiggen i pennelli ANZA? "

Mentre metti in discussione le tue ipotesi e le tue scoperte, scoprirai i pregiudizi e alla fine scoprirai qual è la linea di condotta corretta.

Inizia dal presupposto che tutti i membri del tuo team sono perfettamente razionali e hanno in mente i migliori interessi del team e del prodotto, proprio come te. Se stanno facendo qualcosa che non ha senso, allora devi capire cosa non sai di loro, o cosa sai che loro non sanno.

13 ore non sono un grosso problema, lo farei. Ricorda che vieni pagato per questo. Scrivilo come "sicurezza del lavoro". Inoltre è meglio mantenere un buon karma tra la squadra. Ora, se fosse qualcosa che ti avrebbe richiesto una settimana o più, allora potresti coinvolgere il tuo manager e chiedergli se è il miglior uso del tuo tempo, specialmente se non sei d'accordo.

Tuttavia, sembra che tu abbia bisogno di leva nel tuo gruppo. Ecco come si ottiene leva: chiedere perdono non chiedere il permesso. Aggiungi cose al programma come meglio credi (nell'ambito del corso, cioè assicurati che risolva completamente il problema che il capo vuole ...), e dica al direttore o ai tuoi colleghi dopo il fatto. Non chiedere loro: "Va bene se aggiungo la funzione X". Piuttosto, aggiungi semplicemente le funzionalità che vuoi personalmente nel programma. Se si arrabbiano con una nuova funzione o non sono d'accordo, va bene rimuoverlo. Se gli piace, tienilo.

Inoltre ogni volta che ti chiedono di fare qualcosa, fai il "miglio supplementare" e aggiungi un sacco di cose che hanno dimenticato di menzionare o cose che funzionano meglio di quello che hanno detto. Ma non chiedere loro se è "ok" fare il miglio supplementare. Fallo e casualmente diglielo dopo averlo fatto. Quello che stai facendo è addestrarli ..

Ciò che accadrà sarà il tuo manager che ti collegherà come un "go-getter" e inizierà a fidarsi di te, e i tuoi colleghi inizieranno a vederti come lead bc che stai iniziando a possedere il programma. E poi quando cose come quelle che hai detto succedono in futuro, avrai più voce in capitolo perché sei essenzialmente la star della squadra e i compagni di squadra si tireranno indietro se non sei d'accordo con loro.

La revisione del codice ha diversi scopi. Una cosa di cui sei ovviamente consapevole è " Questo codice è adatto allo scopo? " In altre parole, è funzionalmente corretto; è adeguatamente testato; le parti non ovvie sono opportunamente commentate; è conforme alle convenzioni del progetto?

Un'altra parte della revisione del codice è la condivisione delle conoscenze sul sistema. È un'opportunità sia per l'autore che per il revisore di conoscere il codice modificato e il modo in cui interagisce con il resto del sistema.

Un terzo aspetto è che può fornire una revisione dei problemi esistenti prima che vengano apportate modifiche. Molto spesso, quando rivedo le modifiche di qualcun altro, trovo qualcosa che ho perso in una precedente iterazione (spesso qualcosa del mio). Una frase del tipo "Ecco un'opportunità per renderlo più solido di quanto fosse", non è una critica e non la prenda come una cosa sola!

Il mio team attuale considera la revisione del codice non solo come un gateway o ostacolo che il codice deve passare indenne prima del commit, ma soprattutto come un'opportunità per una discussione piuttosto strutturata di una particolare area di funzionalità. È una delle occasioni più produttive per la condivisione delle informazioni. (E questa è una buona ragione per condividere la revisione del team, piuttosto che sempre lo stesso revisore).

Se percepisci le revisioni del codice come attività conflittuale, è una profezia che si autoavvera. Se invece li consideri come la parte più collaborativa del tuo lavoro, stimoleranno miglioramenti continui al tuo prodotto e al modo in cui lavorerai insieme. Aiuta se una recensione può essere chiara sulle priorità relative dei suoi suggerimenti - c'è un miglio di differenza tra "Mi piacerebbe un commento utile qui" e "Questo si rompe se x è sempre negativo", per esempio.

Avendo fatto un sacco di affermazioni generali sopra, come si applica alla tua situazione specifica? Spero che ora sia ovvio che il mio consiglio è di rispondere alla revisione con domande aperte e di negoziare quale approccio ha più valore. Per il tuo caso esemplificativo in cui viene suggerito un test aggiuntivo, quindi qualcosa come "Sì, possiamo testarlo, credo che occorrerà < time > per implementare. E c'è qualcos'altro che possiamo fare per garantire che il test non sia necessario? "

Una cosa che mi colpisce quando leggo la tua domanda: se ci vogliono due giorni per scrivere un nuovo test, allora il tuo nuovo test è uno scenario molto diverso dai tuoi test esistenti (nel qual caso probabilmente ha un sacco di valore) o hai identificato la necessità di un migliore riutilizzo del codice nella suite di test.

Infine, come commento generale sul valore delle revisioni del codice (e come un conciso riassunto di ciò che ho detto sopra), mi piace questa affermazione, in Maintainers Do not Scale di Daniel Vetter :

At least for me, review isn’t just about ensuring good code quality, but also about diffusing knowledge and improving understanding. At first there’s maybe one person, the author (and that’s not a given), understanding the code. After good review there should be at least two people who fully understand it, including corner cases.

Il codice può SEMPRE essere migliore.

Se sei in una revisione del codice e non vedi nulla che potrebbe essere migliore o un test unitario che potrebbe rilevare un bug, non è il codice perfetto, ma il revisore che non sta facendo il loro lavoro. Se hai scelto di menzionare il miglioramento è una scelta personale. Ma quasi ogni volta che la tua squadra fa una revisione del codice, dovrebbero esserci cose che qualcuno noterà che potrebbe essere migliore o che tutti probabilmente hanno perso tempo.

Detto questo, il fatto che tu agisca sui commenti o meno dipende dalla tua squadra. Se le tue modifiche risolvono il problema o aggiungono valore sufficiente senza modifiche che il tuo team le accetta, allora le uniscono e registrano i loro commenti nel backlog affinché qualcuno possa rivolgersi in seguito. Se il team trova le tue modifiche più rischiose o complesse del valore, devi risolvere i problemi di conseguenza.

Ricorda che qualsiasi codice ha almeno un altro caso limite che potrebbe essere testato e potrebbe utilizzare almeno un altro refactoring. Questo è il motivo per cui le revisioni del codice vengono eseguite al meglio come un gruppo con tutti gli utenti che guardano lo stesso codice allo stesso tempo. In modo che alla fine tutti possano arrivare a un consenso sull'opportunità o meno che il codice in revisione sia accettabile (così com'è) e aggiunge abbastanza valore per unirsi alla base della comunità o se certe cose devono essere fatte prima che ci sia abbastanza valore da unire .

Dato che stai facendo questa domanda presumo che tu non stia facendo realmente "revisioni del codice", ma invece creando una richiesta di pull o un altro meccanismo di sottomissione affinché altri possano commentare in modo non deterministico. Quindi ora ti trovi in un problema di gestione e una definizione di fatto. Direi che la tua gestione è indecisa e in realtà non capisce il processo e lo scopo delle revisioni del codice e probabilmente non ha una "definizione di fatto" (DOD). Perché se lo facessero, il tuo DOD risponderà esplicitamente a questa domanda e non dovresti venire qui a chiedere.

Come lo risolvi? Bene, chiedete al vostro manager di darvi un DOD e dirvi se dovete sempre attuare tutti i commenti. Se la persona in questione è il tuo manager, la risposta è evidente.

Questa domanda non riguarda le virtù della programmazione difensiva, i pericoli dei casi d'angolo, oi rischi catastrofici dei bug nei prodotti fisici. In effetti, non riguarda nemmeno l'ingegneria del software .

In realtà, il modo in cui un giovane professionista gestisce le istruzioni di un terapista senior quando il junior non può essere d'accordo o apprezzarlo.

Ci sono due cose che devi apprezzare per essere uno sviluppatore junior. In primo luogo, significa che mentre è possibile che tu sei nel giusto, e lui nel torto, è - in equilibrio di probabilità - non probabile. Se il tuo collega ti sta suggerendo di non vedere il valore, devi seriamente prendere in considerazione la possibilità che tu non sia abbastanza esperto per comprenderlo. Non ho questo senso da questo post.

La seconda cosa da apprezzare è che il tuo partner senior è così chiamato perché ha più responsabilità. Se un minore rompe qualcosa di importante, non si troveranno nei guai se seguiranno le istruzioni. Se un ha permesso per rompere il problema, tuttavia - non sollevando problemi nella revisione del codice, ad esempio - sarebbero giustamente molto in difficoltà.

In definitiva, è un requisito implicito del tuo lavoro che osservi le istruzioni di coloro che hanno fiducia nell'impresa per guidare i loro progetti. Non riesci generalmente a rimandare agli anziani quando ci sono buone ragioni per valutare la loro esperienza? Hai intenzione di seguire nessuna istruzione che non puoi capire? Pensi che il mondo dovrebbe fermarsi finché non sei convinto? Questi valori non sono compatibili con il lavoro in un team.

Un ultimo punto. Ripensa ai progetti che hai scritto sei mesi fa. Pensa ai progetti che hai scritto all'università. Vedi quanto sono orribili ora - tutti gli insetti e il design sottosopra, i punti ciechi e le astrazioni fuorviate? E se ti dicessi che tra sei mesi valuteresti gli stessi difetti del lavoro che stai facendo oggi? Questo aiuta a dimostrare quanti punti ciechi sono nel tuo attuale approccio? Perché questa è la differenza che fa l'esperienza.

constantly makes comments to my code that suggest me to do a lot more work than is necessary.

Puoi dare consigli, ma alla fine non è il tuo ruolo decidere cosa è necessario. È tuo compito implementare ciò che la direzione o (in questo caso il tuo revisore) decide sia necessario. Se non sei d'accordo con ciò che è necessario troppo o troppo strong, probabilmente ti ritroverai senza lavoro. Di conseguenza è parte della tua professionalità scendere a patti con questo ed essere in pace con esso.

He had suggested that I handle an obscure case that is extremely unlikely to happen

Ci sono altre ottime risposte qui che mostrano come anche i non bug (cioè qualcosa che può non fallire in modo mai ) dovrebbero comunque essere rielaborati a volte. (ad es. in caso di costruzione in futuro della sicurezza del prodotto, in base agli standard, ecc.) Parte del ruolo di un grande sviluppatore è avere la massima fiducia che il tuo codice sarà robusto in ogni situazione immaginabile ogni volta, e in futuro anche protetto, non solo lavorando in situazioni testate nelle condizioni attuali la maggior parte del tempo

Suggerimenti per codificare i revisori per aumentare l'utilità aziendale della revisione del codice (tu come OP dovresti proporre tale modifica):

• Annota i tuoi commenti per tipo. "Critico" / "Must-Do" / "Opzionale" / "Miglioramenti suggeriti" / "Bello da avere" / "Sto meditando".

  Se questo sembra troppo CDO / anale / complicato, almeno utilizza 2 livelli: "È necessario correggere per passare la revisione e poter unire la tua modifica" / "Tutti gli altri".

Suggerimenti per la gestione dei suggerimenti per la revisione del codice che sembrano meno importanti da fare:

• Crea un ticket aperto nel tuo sistema di ticketing preferito (il tuo team ne usa uno, si spera?), seguendo il suggerimento

• Metti il numero di ticket come commento di risposta alla voce di revisione del codice se il tuo processo consente risposte a commenti come Fisheye o recensioni di email.

• Rivolgiti al revisore e chiedi esplicitamente se l'elemento è del tipo "necessario correggere o non essere unito / rilasciato".

  • Se la risposta è "Sì", ma non sei d'accordo, lascia che la persona responsabile della gestione del progetto (PM, il tuo manager, ecc.) prenda una decisione - presenti il disaccordo in modo completo e onesto. Non si tratta di chi di voi ha ragione, ma di ciò che è meglio per il progetto, quindi il lavoro di PM / manager.

Ora tratta questo ticket come qualsiasi altra richiesta di sviluppo.

1. Se viene deciso di essere urgente dopo l'escalation, trattarlo come una richiesta di sviluppo urgente. Deprioretize altri lavori e lavora su questo.

2. Altrimenti, lavoraci secondo la priorità assegnata e il suo ROI (che può differire in base alla tua linea di business come spiegato in altre risposte).

Devi non intensificare questo alla gestione.

Nella maggior parte delle aziende il tipo di gestione sceglierà sempre di non scrivere quel test in più, di non sprecare tempo marginalmente a migliorare la qualità del codice, di non perdere tempo .

In molti casi la qualità del codice dipende dalle regole non scritte nel team di sviluppo e dallo sforzo extra che i programmatori inseriscono.

Sei uno sviluppatore junior e questa è la tua prima revisione del codice . Devi accettare il consiglio e fare il lavoro. Puoi migliorare il flusso di lavoro e le regole del tuo team solo se prima li conosci e li rispetti per un po ', in modo che tu possa capire perché ci sono. Altrimenti sarai quel ragazzo nuovo che non rispetta le regole e diventa il lupo solitario della squadra.

Sei nuovo nel team, segui i consigli che ottieni per un po ', scopri perché sono lì, non portare il primo consiglio che ti viene messo in dubbio durante la riunione di mischia. Le vere priorità di business saranno evidenti dopo un po 'senza chiedere (e potrebbe non essere quello che il manager ti dirà faccia a faccia).

È molto importante creare codice che soddisfi le tue richieste di gestione / lead. Ogni altro dettaglio sarebbe solo un "bello da avere". Se sei un esperto (leggi questo: "se non sei uno sviluppatore junior") nel tuo campo, allora sei "idoneo" ad affrontare problemi minori che trovi lungo il percorso senza consultare i tuoi leader ogni volta.

Se pensi che qualcosa non va e sei relativamente esperto nel tuo campo allora è probabile che tu abbia ragione .

Ecco alcune affermazioni che potrebbero esserti utili:

• Mi è stato chiesto di fare X, il revisore del codice suggerisce anche di fare Y, dovrei farlo o dovrei passare a cose più importanti?
• Mi stai suggerendo Y, quindi puoi capire almeno un caso di test che catturerebbe quel comportamento in modo da poterlo testare? Credo che il codice non verrà chiamato.
• Forse non dovremmo prima sviluppare una soluzione sicura per i casi scoperti? Quindi li prendiamo presto e li sistemiamo in modo che possiamo concentrarci su cose più importanti.
• OK, mentre sto implementando Y, potresti essere così gentile da scrivere alcuni casi di test in modo da ottenere quella cosa una volta per tutte ?
• Mi è stato chiesto di fare X, e penso che potrei fare Y a meno che non ci siano altre priorità. La prossima volta perché non invii una richiesta di funzione invece di inserirla come commento di revisione sul mio codice ? Almeno possiamo ascoltare ulteriori opinioni di altri membri del team su quella funzione prima di implementarla (in genere qualsiasi elemento importante dovrebbe essere una caratteristica e dovrebbe essere gestito da più di una persona, in genere la revisione del codice dovrebbe riguardare la revisione approcci al codice e alle soluzioni, il resto è risoluzione dei bug e funzionalità).

Pensi seriamente che l'atteggiamento del tuo revisore sia dannoso per il progetto o pensi che sia giusto quasi tutte le volte (tranne forse a volte commette errori minori nella valutazione ed esagera quello)?

Per me sembra più che lui stia scrivendo cose che non appartengono a una revisione del codice, il che è una cattiva pratica perché rende più difficile per tutti tenere traccia delle cose. Tuttavia non so quali altri commenti di revisione ha fatto, quindi non posso dire nulla su altri casi.

In generale, cerca di evitare entrambi i seguenti:

• Non stai facendo ciò che è stato richiesto
• Fai sentire stupido il tuo revisore

Se sta effettivamente rivedendo il tuo codice, è perché la gestione si fida di lui più di te.

In caso di disaccordo durante la revisione del codice sull'ambito:

1. Documenta l'ambito effettivamente coperto dal codice. A nessuno piacciono le brutte sorprese.
2. Comprendi che l'ambito è una decisione aziendale. L'ambito dovrebbe già essere noto quando inizi a lavorare su una funzione, ma se non lo è puoi sempre chiedere chiarimenti in seguito.

Se il revisore del codice è quello che prende le decisioni aziendali, può modificare l'ambito in qualsiasi momento, anche durante la revisione del codice, ma non lo fa nel suo ruolo di revisore del codice.

Se non puoi dimostrare che il caso limite è impossibile, devi supporre che sia possibile. Se è possibile, allora è inevitabile che alla fine si verifichi, e prima piuttosto che dopo. Se il caso limite non si è verificato durante il test, potrebbe essere un suggerimento che la copertura del test sia incompleta.

1. Accetta il feedback.
2. Prima di apportare modifiche al codice, fai del tuo meglio per creare un test per il caso limite e vedere se puoi ottenere un errore di test (prova che il problema esiste). Se è impossibile creare un tale caso di test e ottenere un errore di test, allora potrebbe essere in grado di concludere che il caso limite è effettivamente impossibile (anche se sarei esitante a trarre una simile conclusione) .
3. Se riesci a ottenere un errore di test, applica la modifica del codice appropriata per superare il test.

Per il bene del prodotto, probabilmente vorrai essere in grado di produrre il caso limite e indurre un errore, in modo che tu possa applicare la correzione e avere la certezza che è stato evitato un potenziale problema.

L'intero punto delle recensioni del codice è di aggiungere ulteriori occhi al codice. Nessuno di noi è immune da errori o dimenticanze. È fin troppo comune guardare un pezzo di codice molte volte e non notare un errore evidente, in cui un nuovo paio di occhi può captarlo immediatamente.

Come hai detto, hai implementato un nuovo algoritmo. Non sarebbe saggio trarre conclusioni sul comportamento del nuovo algoritmo in base al comportamento o alle osservazioni sul suo predecessore.

Ci sono revisori di codice che sanno cosa stanno facendo e, se dicono che qualcosa deve essere cambiato, devono essere cambiati e quando dicono che qualcosa deve essere testato, allora deve essere testato.

Ci sono revisori del codice che devono giustificare la propria esistenza creando del lavoro inutile per gli altri.

Qual è l'elemento che devi decidere, e come gestire il secondo tipo è più una domanda per workplace.stackexchange.

Se usi la mischia, allora la domanda è se il tuo lavoro fa quello che dovrebbe fare (apparentemente lo fa), e puoi mettere la gestione del caso estremamente raro e forse impossibile nel backlog, dove verrà data la priorità, e se va in uno sprint, allora il tuo revisore può sentirsi libero di prenderlo e fare il lavoro di 13 ore. Se fai un lavoro X e perché lavori X, ti rendi conto che anche il lavoro Y deve essere eseguito, quindi il lavoro Y non diventa parte del lavoro X, è un lavoro indipendente.