I commenti in linea sono 'modificati da' inline nei negozi che usano il controllo di revisione?

17

Lo sviluppatore senior del nostro negozio insiste sul fatto che ogni volta che il codice viene modificato, il programmatore responsabile dovrebbe aggiungere un commento in linea che indichi cosa ha fatto. Questi commenti di solito assomigliano a // YYYY-MM-DD <User ID> Added this IF block per bug 1234.

Utilizziamo TFS per il controllo di revisione e mi sembra che i commenti di questo tipo siano molto più appropriati come note di check-in piuttosto che come rumore in-line. TFS ti consente persino di associare un check-in con uno o più bug. Alcuni dei nostri file di classi più vecchi e spesso modificati sembrano avere un rapporto da commento a LOC che si avvicina a 1: 1. Ai miei occhi, questi commenti rendono il codice più difficile da leggere e aggiungono valore zero.

È una pratica standard (o almeno comune) in altri negozi?

    
posta Joshua Smith 16.02.2011 - 17:25
fonte

8 risposte

23

Solitamente considero tali commenti come una cattiva pratica e penso che questo tipo di informazioni appartenga ai registri di commit SCM. Rende il codice più difficile da leggere nella maggior parte dei casi.

Tuttavia , faccio ancora spesso qualcosa del genere per specifici tipi di modifiche.

Caso 1 - Attività

Se usi un IDE come Eclipse, Netbeans, Visual Studio (o hai qualche modo di fare ricerche di testo sulla base di codice con qualcos'altro), forse il tuo team usa alcuni "commenti tag" o "tag attività" specifici. In tal caso ciò può essere utile.

Di tanto in tanto, quando riesamina il codice, aggiungo qualcosa del tipo:

// TOREVIEW: [2010-12-09 haylem] marking this for review because blablabla

o

// FIXME: [2010-12-09 haylem] marking this for review because blablabla

Uso diversi tag di attività personalizzati che posso vedere in Eclipse nella vista attività per questo, perché avere qualcosa nei registri di commit è una buona cosa ma non abbastanza quando hai un dirigente che ti chiede in una riunione di revisione perché la correzione di errori XY è stato completamente dimenticato e sfuggito. Quindi su questioni urgenti o parti di codice veramente discutibili, questo funge da promemoria aggiuntivo (ma di solito manterrò il commento breve e controllerò i registri di commit perché QUESTO è il motivo del promemoria, quindi non ingombrare troppo il codice).

Caso 2 - Patch di Libs di terze parti

Se il mio prodotto ha bisogno di impacchettare una parte di codice di terze parti come sorgente (o libreria, ma ricostruita dall'origine) perché doveva essere riparata per qualche motivo, documentiamo la patch in un documento separato dove elenchiamo quelli "avvertimenti" per riferimento futuro, e il codice sorgente di solito contiene un commento simile a:

// [PATCH_START:product_name]
//  ... real code here ...
// [PATCH_END:product_name]

Caso 3 - Correzioni non ovvie

Questo è un po 'più controverso e più vicino a ciò che il tuo senior dev sta chiedendo.

Nel prodotto su cui lavoro al momento, a volte (sicuramente non è una cosa comune) abbiamo un commento del tipo:

// BUGFIX: [2010-12-09 haylem] fix for BUG_ID-XYZ

Lo facciamo solo se il bugfix non è ovvio e il codice si legge in modo anomalo. Questo può essere il caso, ad esempio, delle stranezze del browser o delle oscure correzioni dei CSS che è necessario implementare solo perché c'è un errore di un documento in un prodotto. Quindi in generale lo collegheremo al nostro repository di problemi interni, che conterrà quindi il ragionamento dettagliato alla base del bugfix e dei puntatori alla documentazione del bug del prodotto esterno (ad esempio, un avviso di sicurezza per un ben noto difetto di Internet Explorer 6, o qualcosa del genere).

Ma come detto, è abbastanza raro. E grazie ai tag delle attività, possiamo regolarmente esaminarli e controllare se queste strane correzioni hanno ancora senso o possono essere eliminate gradualmente (ad esempio, se abbiamo eliminato il supporto per il prodotto buggato che causa il bug in primo luogo).

Questo in: Un vero esempio di vita

In alcuni casi, è meglio di niente:)

Ho appena trovato un'enorme classe di calcolo statistico nella mia base di codice, in cui il commento dell'intestazione era sotto forma di un log delle modifiche con il solito yadda yadda: revisore, data, bug ID.

All'inizio ho pensato di rottamare, ma ho notato che gli ID dei bug non solo non corrispondevano alla convenzione del nostro attuale tracker di problemi, ma nemmeno corrispondevano a quelli del tracker usato prima di entrare nella compagnia. Così ho provato a leggere il codice e a capire cosa stava facendo la classe (non essendo uno statistico) e ho anche cercato di chiarire questi rapporti sui difetti. Come è successo, erano abbastanza importanti e avrebbero reso la vita del prossimo ragazzo a modificare il file senza conoscerne abbastanza orribile, trattandosi di problemi di precisione minori e casi speciali basati su requisiti molto specifici emessi dal cliente originario allora . In conclusione, se questi non fossero stati lì, non avrei saputo. Se non fossero stati lì dentro E avevo avuto una migliore comprensione della classe, avrei notato che alcuni calcoli erano fuori uso e li avevano "riparati".

A volte è difficile tenere traccia di requisiti molto vecchi come questi. Alla fine quello che ho fatto è stato ancora rimuovere l'intestazione, ma dopo aver nascosto in un commento di blocco prima di ogni funzione compromettente che descrive il motivo per cui questi calcoli "strani" sono richieste specifiche.

Quindi, in quel caso, consideravo ancora queste cattive abitudini, ma ero felice che lo sviluppatore originale lo facesse almeno! Sarebbe stato meglio commentare il codice chiaramente, invece, ma suppongo che sia stato meglio di niente.

    
risposta data 16.02.2011 - 17:47
fonte
3

Usiamo questa pratica per i file che non sono sotto controllo di versione. Abbiamo programmi RPG che girano su un mainframe e si è dimostrato difficile fare molto di più che eseguirne il backup.

Per il codice sorgente con versione, utilizziamo le note per il check-in. Penso che sia il loro posto, non il codice ingombrante. Sono i metadati, dopotutto.

    
risposta data 16.02.2011 - 17:28
fonte
2

Un tempo avevamo fatto questa pratica in un negozio SW dove il nostro manager insisteva che voleva essere in grado di leggere i file di codice su carta e seguire la cronologia delle loro revisioni. Inutile dire che non ricordo alcuna occasione concreta quando ha effettivamente esaminato la stampa di un file sorgente: - /

Fortunatamente, i miei manager da allora sono stati più informati su cosa possono fare i sistemi di controllo delle versioni (devo aggiungere che abbiamo usato SourceSafe in quel primo negozio). Quindi non aggiungo metadati di versione nel codice.

    
risposta data 16.02.2011 - 17:38
fonte
2

Generalmente non è necessario se SCM e IDE ti permettono di usare la funzione "mostra annotazione" (chiamata in questo caso in Eclipse) per vedere facilmente quale commit ha cambiato un pezzo di codice, e i commenti di commit dovrebbero indicare chi e perché.

L'unica volta che ho inserito un commento come questo nel codice è se si tratta di una parte di codice particolarmente strana che potrebbe causare confusione in futuro, quindi commenterò brevemente il numero del bug in modo che possano andare al bug segnala e leggi in dettaglio a riguardo.

    
risposta data 16.02.2011 - 17:51
fonte
2

Ovviamente, tali commenti sono quasi sicuramente non corretti nel tempo; se si modifica una riga due volte, si aggiungono due commenti? Dove vanno? Quindi un processo migliore è fare affidamento sui tuoi strumenti.

Questo è un segno che, per qualsiasi ragione, lo sviluppatore senior non può fare affidamento sul nuovo processo per tenere traccia delle modifiche e connettere le modifiche al sistema di tracciamento dei problemi. Devi affrontare questo disagio per risolvere il conflitto.

Devi capire perché non ci può fare affidamento. Sono vecchie abitudini? Una brutta esperienza con il sistema SCM? Un formato di presentazione che non funziona per lui? Forse non sa nemmeno di strumenti come 'git blame' e la vista timeline di Perforce (che in sostanza lo mostra, anche se potrebbe non mostrare quale problema ha innescato la modifica).

Senza capire la sua ragione per il requisito, non sarai in grado di convincerlo.

    
risposta data 16.02.2011 - 17:51
fonte
2

Lavoro su un prodotto Windows vecchio di 20 anni. Abbiamo una pratica simile che è in vigore da molto tempo. Non riesco a contare il numero di volte in cui questa pratica ha salvato il mio bacon.

Sono d'accordo che alcune cose sono ridondanti. Un tempo gli sviluppatori utilizzavano questa pratica per commentare il codice. Quando ripasso, dico loro di andare avanti ed eliminare il codice, e il commento non è necessario.

Ma, in molte occasioni, puoi guardare il codice che è stato circa un decennio e modificato da 20 sviluppatori, ma mai refactored. Tutti hanno da tempo dimenticato tutti i dettagli dei requisiti che erano nel codice originale, non importava tutte le modifiche e non c'è documentazione affidabile.

Un commento con un numero di difetto mi dà un posto dove andare a cercare l'origine del codice.

Quindi, sì, il sistema SCM fa molto, ma non tutto. Tratta questi come faresti con qualsiasi altro commento e aggiungili ovunque venga apportato un cambiamento significativo. Lo sviluppatore guarda il tuo codice più di 5 anni da come ti ringrazierà

    
risposta data 03.03.2011 - 14:48
fonte
1

Citare ogni cambiamento nei commenti è inutile se hai un VCS. È utile menzionare un bug specifico o un'altra nota importante relativa alla linea specifica . Una modifica può includere molte linee alterate, tutte con lo stesso messaggio di commit.

    
risposta data 16.02.2011 - 17:50
fonte
1

Non che io possa dire.

Io faccio splatter TODOs nel mio codice mentre procedo e l'obiettivo è quello di rimuoverli per tempo di revisione.

    
risposta data 16.02.2011 - 19:32
fonte

Leggi altre domande sui tag