È una cattiva pratica non eliminare i file ridondanti subito da VCS, ma contrassegnarli come "Da eliminare" con i commenti prima?

53

Volevo sapere se il modo in cui gestisco i file di origine che devono essere eliminati dal controllo di versione potrebbe essere considerato una cattiva pratica.

Voglio spiegartelo in base a quell'esempio:

Recentemente mi sono arrabbiata molto perché ho dovuto ordinare noiosamente le classi Java in un programma che era praticamente un codice morto, ma non era stato documentato da nessuna parte e non era nemmeno commentato in quelle classi Java. Ovviamente avevano bisogno di essere cancellati ma prima di eliminare tali elementi ridondanti ho un - qualcuno potrebbe dire strano - abitudine:

Non elimino immediatamente tali file ridondanti tramite SVN- > Delete (sostituisci con il comando delete del tuo sistema di controllo versione preferito), ma inserisco invece commenti in quei file (mi riferisco sia alla testa che al footer) che saranno cancellati + il mio nome + la data e anche - ancora più importante - PERCHÉ SONO CANCELLATI (nel mio caso, perché erano morti, codice confuso). Quindi li salvo e li impegno nel controllo della versione. La prossima volta che devo eseguire il commit / check in qualcosa nel progetto per il controllo della versione, premo SVN- > Delete e poi vengono eliminati in Controllo versione - ancora ovviamente ripristinabili attraverso le revisioni però ed è per questo che ho adottato quell'abitudine .

Perché farlo invece di eliminarli immediatamente?

La mia ragione è che voglio avere marcatori espliciti almeno nell'ultima revisione in cui esistevano quei file ridondanti, perché meritavano di essere cancellati. Se li elimini subito, vengono cancellati ma non viene mai documentato perché sono stati cancellati. Voglio evitare uno scenario tipico come questo:

"Hmm... why were those files deleted? I did work fine before." (Presses 'revert' -> guy who reverted then is gone forever or not available in the next weeks and the next assignee has to find out tediously like me what those files are about)

Ma non noti perché questi file sono stati eliminati nei messaggi di commit?

Ovviamente lo faccio, ma un messaggio di commit a volte non viene letto dai colleghi. Non è una situazione tipica che quando si tenta di comprendere il codice (nel mio caso morto) prima si controlla il registro di controllo della versione con tutti i messaggi di commit associati. Invece di strisciare attraverso il registro, un collega può vedere subito che questo file è inutile. Si salva il suo tempo e lei / lui sa che questo file è stato probabilmente ripristinato per il male (o almeno solleva una domanda.

    
posta Bruder Lustig 14.06.2017 - 18:01
fonte

7 risposte

110

Il problema con l'aggiunta di un commento a un file che dovrebbe essere eliminato, invece di eliminarlo nel controllo del codice sorgente e inserire la spiegazione, è il presupposto che se gli sviluppatori non leggono i messaggi di commit leggeranno sicuramente i commenti nell'origine codice.

Dal punto di vista di un estraneo, questa metodologia sembra essere radicata in una visione molto conservativa del controllo del codice sorgente.

"Che cosa succede se elimino questo file non utilizzato e poi qualcuno ne ha bisogno?" qualcuno potrebbe chiedere.

Stai utilizzando il controllo del codice sorgente. Ripristina la modifica o, meglio ancora, parla con la persona che ha eliminato il file (comunica).

"Che cosa succede se elimino il file morto, quindi qualcuno inizia a usarlo di nuovo e apportano modifiche?" qualcun altro potrebbe chiedere.

Di nuovo, stai utilizzando il controllo del codice sorgente. Otterrai un conflitto di fusione che una persona deve risolvere. La risposta qui, come con l'ultima domanda, è comunicare con i tuoi compagni di squadra.

Se davvero dubiti che un file debba essere rimosso, comunica prima cancellandolo dal controllo del codice sorgente. Forse solo di recente ha smesso di essere usato, ma una funzione imminente potrebbe averne bisogno. Non lo sai, ma uno degli altri sviluppatori potrebbe.

Se deve essere rimosso, rimuoverlo. Taglia il grasso dal codice base.

Se hai fatto una "oopsie" e hai effettivamente bisogno del file, ricorda che stai usando il controllo del codice sorgente per poter recuperare il file.

Vincent Savard, in un commento sulla domanda, ha detto:

... If your colleagues don't read the commit messages and they resurrect a file that was rightfully deleted and it passes code reviewing, there's definitely something wrong in your team, and it's a great opportunity to teach them better.

Questo è un buon consiglio. Le recensioni del codice dovrebbero catturare questo genere di cose. Gli sviluppatori devono consultare i messaggi di commit quando viene apportata una modifica inaspettata a un file o un file viene rimosso o rinominato.

Se i messaggi di commit non raccontano la trama, anche gli sviluppatori devono scrivere messaggi di commit migliori.

Avere paura di cancellare il codice o eliminare i file è indicativo di un problema sistemico più profondo con il processo:

  • Mancanza di recensioni accurate del codice
  • Mancanza di comprensione su come funziona il controllo del codice sorgente
  • Mancanza di comunicazione tra team
  • Messaggi di commit scadenti da parte degli sviluppatori

Questi sono i problemi da affrontare, quindi non hai voglia di lanciare sassi in una serra quando elimini codice o file.

    
risposta data 14.06.2017 - 18:54
fonte
105

Sì, è una cattiva pratica.

Dovresti inserire la spiegazione dell'eliminazione nel messaggio di commit quando esegui il commit dell'eliminazione dei file.

I commenti nei file sorgente dovrebbero spiegare il codice così com'è attualmente . I messaggi di commit dovrebbero spiegare perché sono state apportate le modifiche nel commit, quindi la cronologia del commit sul file spiega la sua cronologia.

Scrivere commenti che spieghino i cambiamenti direttamente nella fonte stessa renderà il codice molto più difficile da seguire. Pensaci: se commenti il file ogni volta che cambi o cancelli il codice, presto i file verranno sommersi di commenti sulla cronologia delle modifiche.

    
risposta data 14.06.2017 - 18:06
fonte
15

Secondo me entrambi delle tue opzioni sono non ottimali , contro i cattivi , pratica:

  1. L'aggiunta di commenti aggiunge valore solo se qualcuno legge questi commenti.
  2. Semplicemente cancellando dal VCS, (con un motivo nella descrizione del cambiamento), può influire su un risultato critico e molte persone non leggono la descrizione del cambiamento, specialmente quando sono sotto pressione.

La mia pratica preferita - in gran parte dovuta al fatto di essere stata morsa più volte nel corso degli anni, tenendo presente che non si sa sempre da dove o da chi viene usato il codice - è a:

  1. Aggiungi un commento affermando che si tratta di un candidato per l'eliminazione e una deprecazione #warning o simile, (la maggior parte delle lingue ha un qualche tipo di meccanismo, per esempio in Java , nel peggiore dei casi è sufficiente un'istruzione print o simile), idealmente con una sorta di scala temporale e con i dettagli di contatto. Ciò avviserà chiunque sia ancora utilizzando il codice. Questi avvertimenti sono normalmente all'interno di ogni funzione o classe se tali elementi esistono .
  2. Dopo un po 'di tempo aggiorna l'avviso a un file standard scope #warning (alcune persone ignorano gli avvisi di deprecazione e alcune catene di strumenti non visualizzano tali avvertimenti per impostazione predefinita).
  3. Successivamente sostituisci l'ambito del file #warning con #error o equivalente - questo interromperà il codice in fase di compilazione ma, se necessario, verrà rimosso, ma la persona che lo rimuove non può ignorarlo. (Alcuni sviluppatori non leggono né indirizzano alcun avvertimento o ne hanno così tanti da non poter separare quelli importanti).
  4. Contrassegna infine il / i file, (o dopo la data di scadenza), come cancellato nel VCS.
  5. Se si elimina un intero pacchetto / libreria / ecc. nella fase di avviso, aggiungerei un README.txt o README.html o un simile con le informazioni su quando & perché è pianificato di sbarazzarsi del pacchetto e lasciare solo quel file, con una modifica per dire quando è stato eliminato, come l'unico contenuto del pacchetto per qualche tempo dopo aver rimosso il resto del contenuto.

Un vantaggio di questo approccio è che alcuni sistemi di versioning (CVS, PVCS, ecc.) non rimuoveranno un file esistente al checkout se è stato cancellato nel VCS. Fornisce inoltre agli sviluppatori coscienziosi, coloro che risolvono tutti i loro avvertimenti, un sacco di tempo per risolvere il problema o fare ricorso alla cancellazione. Inoltre costringe gli sviluppatori rimanenti a guardare almeno l'avviso di cancellazione e si lamentano molto .

Si noti che #warning / #error è un meccanismo C / C ++ che causa un avvertimento / errore seguito dal testo fornito quando il compilatore incontra quel codice, java / java-doc ha @Deprecated annotazione a emettere un avviso sull'uso & @deprecated per fornire alcune informazioni, ecc. ci sono ricette fare simili in python e amp; Ho visto assert utilizzato per fornire informazioni simili a #error nel mondo reale.

    
risposta data 14.06.2017 - 20:46
fonte
3

Sì, direi che è una cattiva pratica, ma non perché è nei file rispetto al messaggio di commit. Il problema è che stai tentando di apportare modifiche senza comunicare con il tuo team.

Ogni volta che apporti modifiche al bagagliaio - aggiungendo, eliminando o modificando i file in qualsiasi modo - dovresti, prima di reinserire nel bagagliaio, parlare di queste modifiche direttamente con un membro ( o membri) della squadra che sarebbe direttamente a conoscenza di loro (in sostanza, discutere di cosa si metterebbe in quelle intestazioni direttamente con i propri compagni di squadra). Ciò assicurerà che (1) il codice che stai cancellando abbia davvero bisogno di essere cancellato, e che (2) il tuo team avrà molte più probabilità di sapere che il codice è stato cancellato e quindi non provare a ripristinare le tue cancellazioni. Senza contare che avrai anche il rilevamento dei bug e simili per il codice che aggiungi.

Ovviamente, quando cambi cose importanti, dovresti inserirle anche nel messaggio di commit, ma poiché hai già discusso, non è necessario che sia la fonte di annunci importanti. La risposta di Steve Barnes affronta anche alcune buone strategie da utilizzare se il potenziale pool di utenti per il tuo codice è troppo grande (es. utilizzando i marcatori Deprecated della tua lingua invece di eliminare i file all'inizio.

Se non vuoi andare così a lungo senza commettere (cioè il tuo cambiamento ha più senso come diverse revisioni), è una buona idea creare un ramo dal tronco e impegnarsi nel ramo, quindi unire nuovamente il ramo tronco quando il cambiamento è pronto. In questo modo, il VCS sta ancora eseguendo il backup del file per te, ma le tue modifiche non influiscono in alcun modo sul trunk.

    
risposta data 15.06.2017 - 09:20
fonte
1

Un problema correlato da progetti che si basano su più piattaforme e configurazioni. Solo perché I stabilisco che è morto non significa che sia una determinazione corretta. Nota che il dead in use potrebbe ancora significare dipendenza rudimentale che deve ancora essere eliminata, e alcuni potrebbero essersi persi.

Quindi (in un progetto C ++) ho aggiunto

#error this is not as dead as I thought it was

E l'ho verificato. Aspetta che riesca a ricostruire tutte le piattaforme normali e che nessuno si lamenti a riguardo. Se qualcuno lo trovasse, sarebbe facile rimuovere una riga anziché essere disorientato con un file mancante.

Sono d'accordo, in linea di principio, che i commenti che menzioni stiano duplicando la funzione che dovrebbe essere eseguita nel sistema di gestione delle versioni . Ma potresti avere motivi specifici per supplimentarlo. Non sapere lo strumento VCS non è uno di questi.

    
risposta data 16.06.2017 - 20:59
fonte
-1

Nel continuum delle cattive pratiche, questo è piuttosto secondario. Lo farò occasionalmente, quando voglio controllare un ramo e poter vedere il vecchio codice. Questo può facilitare il confronto con il codice sostitutivo. Di solito ricevo un commento sulla revisione del codice "cruft". Con una piccola spiegazione, passo la revisione del codice.

Ma questo codice non dovrebbe vivere a lungo. Generalmente cancello all'inizio del prossimo sprint.

Se hai colleghi che non leggono i messaggi di commit o non ti chiedono perché hai lasciato il codice nel progetto, allora il tuo problema non è uno stile di programmazione errato. Hai un problema diverso.

    
risposta data 18.06.2017 - 14:36
fonte
-3

puoi anche rifattorizzare la classe e rinominarla con un prefisso a UNUSED_Classname prima di estrarre lo stunt. Quindi dovresti anche impegnare direttamente l'operazione di cancellazione

  • Passaggio 1: rinomina classe, aggiungi il tuo commento, conferma
  • Passaggio 2: elimina il file e effettua il commit

Se è un codice morto, cancellalo. Chiunque ne abbia bisogno, può tornare indietro, ma il passaggio di rinomina impedirà loro di utilizzarlo direttamente senza pensare.

Insegna anche alle persone come guardare tutti i messaggi di commit per un file, alcune persone non sanno nemmeno che è possibile.

    
risposta data 15.06.2017 - 05:34
fonte

Leggi altre domande sui tag