Come devono essere commessi i cambiamenti non funzionali?

8

Sto lavorando su una base di codice legacy piccola e media e quando lavoro su un ticket mi imbatterò in codice che dovrebbe essere ripulito o che ho bisogno di ripulire solo per essere in grado di comprendere il seguito dell'applicazione.

Un esempio reale è:

if( !a && b ){
     doSomething1();
     doSomething2();
     doSomething3();
     doSomething4();
     doSomething5();
}else if( a ){
     doSomething1();
     doSomething2();
     doSomething3();
     doSomething4();
     doSomething5();
}else if( !b && ( a || c )  ){
     doSomething1();
     doSomething2();
     doSomething3();
     doSomething4();
     doSomething5();
}

Un altro sta correggendo gli errori di battitura e Engrish nei commenti e nella documentazione su una dozzina di file sorgente.

Tuttavia, spesso questa pulizia non è correlata al problema principale e mi chiedo come sia meglio impegnare la pulizia. Per come la vedo io, ci sono tre opzioni:

  1. Prima della correzione: funziona in ordine cronologico poiché questo è l'ordine in cui si verificano ma se si rompono qualcosa complicano la correzione e rende più difficile diffondere la correzione con ciò che era in produzione. Introduce anche un commit aggiuntivo che è noise.
  2. Con la correzione: ma ciò oscura la sostituzione effettiva del codice difettoso con i file in cui findGeoragphy era corretta a findGeography .
  3. Dopo la correzione: questo richiede la rimozione e la pulizia che hai fatto per aiutarti a capire il codice e poi ripetere la correzione, commettere la correzione e poi tornare indietro e ripetere la pulizia. Ciò consente la più pulita diff per il codice difettoso, ma duplica lo sforzo e può portare a commit spurie pure.

tl; dr: Quindi, qual è il metodo migliore per eseguire il codice di pulizia?

Contesto: non abbiamo test unitari e proventi dello sviluppo eseguendo le modifiche e osservandole con occhio e gettandole oltre il muro al QA per convalidare le correzioni manuali della regressione. Inoltre, se non eseguiamo alcun tipo di pulizia, il codice diventa inintelligibile. So che questo è tutt'altro che ideale, ma questo è un vero sviluppo aziendale che mette il cibo sul mio tavolo e non la mia scelta.

    
posta ArtB 16.01.2013 - 17:19
fonte

3 risposte

10

Seguo un processo simile alla risposta di Karl. Preferisco un commit separato del cleanup / refactoring prima delle modifiche alle funzionalità per alcuni motivi:

  • Separare i due commit spiegherà il tuo processo e penserà meglio agli altri che guardano i log.
  • Fornisce una serie specifica di modifiche utili durante la revisione del codice.
  • Significa che se si desidera ripristinare le modifiche apportate alle funzionalità si mantiene comunque il cleanup / refactor.
  • Le modifiche alla pulizia possono essere testate separatamente e prima degli aggiornamenti delle funzionalità.
risposta data 16.01.2013 - 17:49
fonte
6

Preferisco il commit extra prima della correzione. Questo separa chiaramente i due compiti che stai facendo e consente ad altre persone di vedere cosa è stato ripulito rispetto a quello che era una correzione di bug. Inoltre, cleanup (almeno per me) di solito è molto più veloce, quindi se eseguo una correzione, non devo preoccuparmi tanto di qualcuno che fa un cambiamento difficile da unire mentre sto lavorando più a lungo compito.

I hanno usato l'approccio "commit with the fix" in luoghi con policy che richiedono un ID richiesta di modifica con ogni commit e non consentono agli sviluppatori di creare nuove richieste di modifica solo per il codice di pulizia . Sì, tali politiche sono improduttive, ma i luoghi di lavoro come questo esistono.

    
risposta data 16.01.2013 - 17:40
fonte
0

Se la correzione potrebbe dover essere corretta in una versione esistente, è necessario prima controllare la correzione e poi eseguire la pulizia, oppure sei (almeno una parte del tempo) che rende la persona che ha bisogno di applica il lavoro di correzione più difficile di quello che è necessario (o per capire cosa è cambiato, o per correggere le modifiche estetiche prima di correggere la correzione reale (noterò che ho apportato più di alcune modifiche esteticamente allettanti che hanno finito per rompere qualcosa, quindi rattoppare più del minimo cambiamento per correggere un difetto può aumentare il rischio di rilasciare qualcosa di brutto).

Sì, questo è un lavoro extra. Sì, è un dolore. Ma sì, è il modo giusto di fare le cose. Il mio normale flusso di lavoro consiste nel fare tutto in una sandbox (pulizia e correzione dei difetti), quindi creare una nuova sandbox, applicare solo la minima correzione di correzione dei difetti, controllare la modifica nella sandbox originale e controllare la pulizia codice. Non è molto lavoro extra, nella mia esperienza.

    
risposta data 16.01.2013 - 18:04
fonte

Leggi altre domande sui tag