Devo insistere che eseguiamo le revisioni del codice prima di unirle nuovamente al trunk?

Sono stato in situazioni simili prima e imho dipende da "devo mantenere il codice".

Se devo mantenere il codice, di quanto voglio un codice di alta qualità, personalmente non richiedo revisioni del codice per ogni commit (i programmatori possono decidere da soli se un determinato codice richiede una revisione o meno) ma se la leggibilità / la manutenibilità soffre di quanto potrebbe essere in ordine.

Durante la lettura:

My concern is that the other developers write ugly code: often breaking encapsulation, writing huge classes, adding internal classes at strange places, having few or no unit tests, and so on. It will eventually be impossible to develop the tool further.

Penso che il tuo problema sia più ampio delle semplici revisioni del codice. Sembra che manchino alcune linee guida e / o non siano implementate. Non avere / pochi test di unità potrebbe essere una cattiva idea, ma dipende dal caso specifico. Tuttavia, breaking encapsulation, writing huge classes, ... rende veramente un codice soggetto a bug, quindi dovrebbe essere risolto definitivamente.

Penso che le revisioni del codice e il mantenimento di alcune linee guida di codifica siano una buona idea, ma penso che farlo per ogni controllo sia una perdita di tempo. È una buona idea quando si crea una squadra e con i giovani programmatori, ma i programmatori esperti possono pensare da soli e alla fine dovrai fidarti di loro. Detto questo, puoi aggiornare periodicamente le recensioni del codice, ma osservare ogni singola riga di codice che entra nel tuo VCS è davvero esagerata.

E un piccolo commento sulle correzioni del tuo collega - alcune volte una brutta soluzione è la soluzione corretta . Può essere che questo particolare codice non sia abbastanza importante per investire molto tempo, può essere che la soluzione semplice sia abbastanza buona ed è meglio investire tempo in altre cose. Rendere il tuo codice "carino" non è il tuo obiettivo principale come programmatore. Il tuo obiettivo principale è quello di offrire e ossessionare su ogni riga di codice semplicemente non ti porterà lì .

Quello che sto cercando di dire è che devi scegliere le tue battaglie. Va bene perdere la battaglia su qualche insignificante classe di utilità per vincere la guerra di consegna (o questa guerra del sottosistema DAVVERO IMPORTANTE, per quella materia).

Se il programma in questione non è un prototipo monouso, penso che le recensioni del codice dovrebbero essere obbligatorie per ogni check-in.

Gli sviluppatori senior potrebbero avere il privilegio di effettuare check-in non revisionati, una volta che sono noti per il loro livello di coscienza da richiedere una revisione quando appropriato.

Non è certo che la revisione del codice sia la risposta fino a quando qualcuno non inizierà ad applicare standard di codifica migliori. Qualcuno scrive codice scadente, commenta / lo ammette e lo controlla in ogni caso. A cosa serve una recensione se qualcuno vuole rifiutare il codice, ma ti mette in ritardo?

Avrai bisogno di: fissare degli standard, supervisionare i principali colpevoli più da vicino e sbilanciarti nella testa che un buon codice non richiede sempre più tempo per scrivere. Devono smettere di usare le scadenze come scusa per rifiutarsi di cambiare le cattive abitudini.