Come rifiutare una revisione del codice che ritieni non sia necessaria?

Richiedi un caso di test che non riesce senza la modifica che ha esito positivo con la modifica.

Se non può produrne uno, lo usi come giustificazione.

Se può produrne uno, devi spiegare perché il test non è valido.

I revisori dovrebbero essere obiettivi.

È chiaro che hai espresso un'opinione sul codice in questione prima ancora di averlo esaminato, e sembra che tu e il fixer abbiate messo in posizione le posizioni. Se è così, allora avrai un momento difficile apparire obiettivo, e un obiettivo ancora più difficile essere . Niente di tutto questo aiuta il processo, e può darsi che la cosa migliore e più obiettiva che puoi fare sia espellere con la motivazione che sei troppo vicino al problema.

Considera un approccio di squadra.

Se non è possibile rimuovere te stesso, forse puoi avere diversi altri ingegneri che esaminano il codice allo stesso tempo. O saranno d'accordo con te che il codice dovrebbe essere rifiutato o loro no. Se sono d'accordo con te, allora non sarà più solo tu contro il fixer, e sarai in grado di dimostrare che il team ha esaminato la correzione obiettivamente e ha deciso di non accettarlo. D'altra parte, se decidono di accettare la correzione, anche quella sarà una decisione della squadra. Dovrebbe essere ovvio dire che dovresti partecipare con una mentalità aperta come puoi gestire, e che non dovresti cercare di influenzare le opinioni degli altri membri del team da qualcosa di diverso da una discussione razionale. Importante: in caso di esito negativo, non buttare la squadra sotto il bus dicendo "Bene I ho sempre detto che era un codice errato, ma ero in inferiorità numerica rispetto agli altri membri del team."

I rifiuti sono una parte naturale del processo di revisione del codice.

Il processo di revisione del codice non è lì per le correzioni del timbro di gomma di persone più anziane; è lì per proteggere e migliorare la qualità del codice. Non c'è niente di sbagliato nel rifiutare una correzione a patto che tu lo faccia per il giusto motivo, cioè che la correzione non migliora il codice. Se, dopo una revisione del codice aperta alla mente, ritieni comunque che la correzione non riduca il rischio e / o la magnitudo di un problema dimostrabile, allora dovresti rifiutarla. Non è personale, solo la tua opinione onesta. Se il fixer non è d'accordo, va bene anche a quel punto e diventa un problema per il management capire. Assicurati di rimanere onesto, aperto e professionale.

La responsabilità taglia in entrambe le direzioni.

Hai detto che non vuoi essere responsabile di questo cambiamento, apparentemente perché non credi che ci sia un problema. Tuttavia, è necessario rendersi conto che se si sbaglia e c'è è un problema, allora si può finire per essere responsabile del rifiuto del codice che avrebbe evitato il problema.

Prendere appunti.

Mantenere un registro scritto del processo di revisione ti aiuterà a mantenere i tuoi fatti dritti. Annota i tuoi pensieri e preoccupazioni durante la revisione, la descrizione e i risultati di eventuali test che potresti eseguire per misurare il presunto problema e la correzione, ecc. Se il problema si intensifica, avrai una registrazione di ciò che hai fatto per supportare il tuo posizione. Se la questione si ripresenta in futuro (probabilmente lo farà se il fixer è collegato alla sua vista), avrai qualcosa da maneggiare nella tua memoria.

Annota i motivi del rifiuto e le difese per probabili argomenti contrari. Quindi discutere razionalmente con il fixer e, se necessario, passare alla gestione.

Se disponi di una documentazione sufficiente (inclusi elenchi di codici, risultati dei test e argomenti obiettivo ), sarai ben coperto.

Provochi qualche malevolenza con il fixer, quindi assicurati che il problema ne valga la pena (cioè, il non-fix causa un danno?)

Inoltre, se il fixer è in qualche modo correlato ai proprietari, dimentica questa risposta.

Recentemente nelle notizie di LinkedIn, c'era un articolo sulla risoluzione dei conflitti sul posto di lavoro e l'umiltà, piuttosto che apparire arroganti. Purtroppo, non riesco a trovare il collegamento ora, ma l'idea generale era di cercare di formulare domande in modo non conflittuale. Per favore, non prendere questo come me sottintendendo che sei arrogante, è solo il modo in cui è stato scritto l'articolo.

In ogni caso, nel dire al programmatore anziano che ha torto nella sua ipotesi, porterà solo a lui un approccio difensivo e ti attaccherà nella sua risposta. Tuttavia, se poni la domanda sul perché crede che il problema esista, dal punto di vista della tua mancanza di comprensione, allora è probabile che sia più aperto a discutere la questione.

Quindi, piuttosto che dire "Il problema non esiste, quindi non dovrei doverlo rivedere", magari chiedere qualcosa del tipo "Per poterlo esaminare correttamente, ho bisogno di capire il problema. dal tuo punto di vista? "

Ad esempio, l'articolo descriveva un test dato ai bambini in cui un adulto teneva un cubo i cui volti erano tutti di un colore, tranne quello rivolto verso l'adulto. Quando ai bambini è stato chiesto quale fosse il colore che l'adulto vedeva, quelli con meno di 5 anni avrebbero quasi sempre detto il colore che potevano vedere, poiché non potevano comprendere che l'adulto potesse avere una visione diversa dalla propria.

C / C ++ può essere pieno di comportamenti indefiniti. Da una parte è male in quanto può portare a comportamenti imprevisti. D'altra parte consente l'ottimizzazione aggressiva e di solito quando si utilizza C / C ++ si è interessati alla velocità.

Scrivere un caso di test che può essere difficile da interrompere - potrebbe implicare un'architettura o un compilatore sconosciuto che non esiste più. Potrebbe anche sembrare come in qualsiasi architettura ragionevole "non dovrebbe causare problemi".

Tuttavia a un certo punto o in un altro cambierai piattaforma (per esempio, vuoi andare su mobile in modo da portarti su ARM o vuoi accelerare le cose e usare GPU). A questo punto le cose potrebbero iniziare a rompersi e sarà necessario eseguirne il debug. Potrebbe essere banale come aggiornare il compilatore alla versione più recente (e potresti volerlo / averne bisogno).

Il codice problematico era:

int d[16];

int SATD (void)
{
   int satd = 0, dd, k;
   for (dd=d[k=0]; k<16; dd=d[++k]) {
     satd += (dd < 0 ? -dd : dd);
   }
   return satd;
 }

Si accede all'ultima iterazione d[++k] => d[++15] => d[16] . Poiché di solito l'elemento successivo era la memoria legale (in termini di paging non modello di memoria C) anche i compilatori banali non producevano alcun eseguibile con un comportamento strano. L'unico modo per scrivere il caso di test era trovare una piattaforma con esattamente lo stesso modello di memoria di C (probabilmente VM).

Tuttavia, la prerelase di gcc 4.8 ha visto che d[++k] viene eseguito in ogni ciclo. Quindi k < 16 altrimenti l'accesso sarebbe illegale e la legalità del programma alimentata al compilatore è parte del contratto. Quindi la condizione del ciclo è sempre vera, date le ipotesi, quindi è un ciclo infinito. Questo potrebbe sembrare strano, ma si trattava di un'ottimizzazione perfettamente legale - l'emissione di system("dd if=/dev/zero of=/dev/sda"); system("format c:") sarebbe anche la corretta sostituzione del ciclo. Esistono modi più sottili per scegliere di mostrare comportamenti. Ad esempio durante la lezione su Memoria transazionale ricordo che il presentatore ha provato diverse volte a ottenere un valore errato quando 2 thread incrementavano lo stesso valore.

Tuttavia C / C ++, come opposto ad alcune lingue, è standardizzato, quindi tale controversia può essere fatta con riferimento alla fonte obiettiva:

• Se pensi che questo non sia un problema prova a dimostrarlo usando lo standard C / C ++ (cioè che porta a valori e comportamenti definiti)
• Se pensa che questo è un problema chiedigli di dimostrarlo usando lo standard C / C ++ (cioè che porta a un valore o comportamento non definito)

In generale, se la tua squadra sta scrivendo in C / C ++ è utile avere uno standard a portata di mano - anche gli esperti di squadra possono trovare qualcosa di strano lì una volta ogni tanto.

Questo sembra più un problema con il criterio del tuo team che con questa recensione specifica. Quando lavoravo in un grande negozio di software in una squadra di 9 persone, un revisore aveva diritto al veto sul codice, e questo era uno standard che tutti noi rispettavamo. Eravamo abbastanza talentuosi e abbastanza esperti - vale a dire. "maturo", ma più nel senso di "non infantile" che "stagionato" - che il nostro team leader potrebbe ragionevolmente aspettarci di essere sempre in grado di raggiungere un accordo senza devolvere argomenti in un periodo di tempo prudente.

Basandomi semplicemente sulla tua lingua nel tuo post, potrei avvertirti che sembra che tu affronterai questa situazione in un modo che potrebbe portare a un argomento devoluto. Forse è "la masturbazione intellettuale", ma probabilmente c'è una ragione per cui l'ha fatto, e il peso per te sarà quello di trovare un modo più semplice per risolvere quel problema - e se non esiste un tale modo, documenta nel commento perché il codice masturbatorio era, infatti, necessario.

Fai in modo che il mittente mostri che il suo codice risolve il suo problema. Se può testare e mostrarti delle prove, allora sei tu a sbagliare e dovresti semplicemente smettere di lamentarti e affrontarlo. Se non è in grado di mostrare le prove a sostegno della sua affermazione, c'è un problema e mostra la prova che la sua patch risolve il problema, quindi lo rimando al tabellone.

Ovviamente potrebbe esserci una politica interna sensibile attorno a questo. Ma questo è il modo in cui andrei (deliziosamente).