Βέλτιστες πρακτικές αναθεώρησης κώδικα

Το Διαδίκτυο παρέχει πληθώρα στοιχείων σχετικά με τις αναθεωρήσεις κώδικα: την επίδραση των σχολίων κώδικα στην εταιρική κουλτούρα, τις επίσημες ανασκοπήσεις ασφαλείας, τους βραχύτερους οδηγούς, τους μεγαλύτερους πίνακες ελέγχου, τις εξανθρωπισμένες αναθεωρήσεις, τους λόγους για τους οποίους γίνεται η ανασκόπηση κώδικα, οι πρακτικές, οι στατιστικές σχετικά με την αποτελεσματικότητα του ελέγχου κώδικα για την αλίευση σφαλμάτων και τα παραδείγματα των αναθεωρήσεων κώδικα έκαναν λάθος. Ω, και φυσικά υπάρχουν και βιβλία. Μεγάλη ιστορία σύντομη, αυτή η θέση στο blog παρουσιάζει τον Palantir για αναθεώρηση κώδικα. Οργανισμοί με βαθιά πολιτισμική απροθυμία να κάνουν αξιολογήσεις από ομότιμους χρήστες ενδέχεται να θέλουν να συμβουλευτούν το εξαιρετικό δοκίμιο του Karl E. Wiegers για την Ανθρωπιστική αξιολόγηση από ομοτίμους πριν προσπαθήσουν να ακολουθήσουν αυτόν τον οδηγό.

Αυτή η ανάρτηση αντιγράφεται από τους οδηγούς βέλτιστων πρακτικών της αλυσίδας εργαλείων Java Code Quality Baseline και καλύπτει τα ακόλουθα θέματα:

  • Γιατί, τι, και πότε πρέπει να κάνετε κριτικές κώδικα
  • Προετοιμασία κώδικα για αναθεώρηση
  • Εκτέλεση ανασκοπήσεων κώδικα
  • Παραδείγματα αναθεώρησης κώδικα

Κίνητρο

Εκτελούμε κριτικές κώδικα (CRs) προκειμένου να βελτιώσουμε την ποιότητα του κώδικα και να επωφεληθούμε από τις θετικές επιδράσεις στην κουλτούρα της ομάδας και της εταιρείας. Για παράδειγμα:

  • Οι υπεύθυνοι της επιχείρησης υποκινούνται από την έννοια ενός συνόλου κριτών που θα εξετάσουν το αίτημα αλλαγής: ο υπεύθυνος τείνει να καθαρίσει τα χαλαρά άκρα, να ενοποιήσει τους TODOs και γενικά να βελτιώσει τη δέσμευση. Η αναγνώριση της εμπειρογνωμοσύνης κωδικοποίησης μέσω συνομηλίκων αποτελεί πηγή υπερηφάνειας για πολλούς προγραμματιστές.
  • Η ανταλλαγή γνώσεων βοηθάει τις ομάδες ανάπτυξης με διάφορους τρόπους:
    - Μια CR ρητά ανακοινώνει την προστιθέμενη / τροποποιημένη / αφαιρεθείσα λειτουργικότητα στα μέλη της ομάδας που μπορούν στη συνέχεια να βασιστούν στο έργο που έχει γίνει.
    - Ο υπεύθυνος μπορεί να χρησιμοποιήσει μια τεχνική ή έναν αλγόριθμο που μπορούν να μάθουν οι κριτικοί. Γενικότερα, οι αναθεωρήσεις κώδικα συμβάλλουν στην αύξηση της γραμμής ποιότητας σε ολόκληρο τον οργανισμό.
    - Οι αναθεωρητές ενδέχεται να έχουν γνώση σχετικά με τις τεχνικές προγραμματισμού ή τη βάση δεδομένων που μπορεί να συμβάλει στη βελτίωση ή στην εδραίωση της αλλαγής. για παράδειγμα, κάποιος άλλος μπορεί να εργάζεται ταυτόχρονα σε ένα παρόμοιο χαρακτηριστικό ή να επιδιορθώσει.
    - Η θετική αλληλεπίδραση και επικοινωνία ενισχύει τους κοινωνικούς δεσμούς μεταξύ των μελών της ομάδας.
  • Η συνέπεια σε μια βάση κώδικα καθιστά τον κώδικα πιο εύκολο να διαβαστεί και να κατανοηθεί, βοηθά στην πρόληψη σφαλμάτων και διευκολύνει τη συνεργασία μεταξύ τακτικών και μεταναστευτικών ειδών προγραμματιστών.
  • Η αναγνωσιμότητα των θραυσμάτων κώδικα είναι δύσκολο να κριθεί για τον συγγραφέα του οποίου το παιδί του εγκεφάλου είναι, και είναι εύκολο να κρίνουμε για έναν αναθεωρητή που δεν έχει το πλήρες πλαίσιο. Ο ευανάγνωστος κώδικας είναι πιο επαναχρησιμοποιήσιμος, χωρίς σφάλματα και ανθεκτικός στο μέλλον.
  • Τα τυχαία σφάλματα (π.χ. τυπογραφικά λάθη) καθώς και διαρθρωτικά σφάλματα (π.χ. νεκρό κώδικα, σφάλματα λογικού ή αλγορίθμου, προβλήματα επιδόσεων ή αρχιτεκτονικής) είναι συχνά πολύ πιο εύκολα εντοπισμένα για κριτικούς κριτές με εξωτερική προοπτική. Μελέτες έχουν διαπιστώσει ότι ακόμη και σύντομες και άτυπες κριτικές κώδικα έχουν σημαντική επίδραση στην ποιότητα του κώδικα και τη συχνότητα των σφαλμάτων.
  • Τα περιβάλλοντα συμμόρφωσης και κανονιστικών ρυθμίσεων συχνά απαιτούν αναθεωρήσεις. Οι CR είναι ένας πολύ καλός τρόπος για να αποφύγετε κοινές παγίδες ασφαλείας. Εάν το χαρακτηριστικό ή το περιβάλλον σας έχει σημαντικές απαιτήσεις ασφαλείας, θα επωφεληθεί (και ίσως χρειαστεί) από τους τοπικούς συνοδούς ασφάλειας (ο οδηγός του OWASP αποτελεί καλό παράδειγμα της διαδικασίας).

Τι να επανεξετάσει

Δεν υπάρχει αληθινά αληθινή απάντηση σε αυτή την ερώτηση και κάθε ομάδα ανάπτυξης θα πρέπει να συμφωνήσει στη δική της προσέγγιση. Ορισμένες ομάδες προτιμούν να αναθεωρούν κάθε αλλαγή που έχει συγχωνευθεί στον κύριο κλάδο, ενώ άλλοι θα έχουν ένα όριο "τριβής", βάσει του οποίου δεν απαιτείται αναθεώρηση. Το συμβιβασμό είναι μεταξύ της αποτελεσματικής χρήσης του χρόνου των μηχανικών (τόσο των συγγραφέων όσο και των κριτών) και της διατήρησης της ποιότητας του κώδικα. Σε ορισμένα ρυθμιστικά περιβάλλοντα, η αναθεώρηση κώδικα μπορεί να απαιτείται ακόμη και για ασήμαντες αλλαγές.

Οι αναθεωρήσεις κώδικα είναι άτακτες: η ύπαρξη του ανώτερου υπευθύνου στην ομάδα δεν σημαίνει ότι ο κωδικός σας δεν χρειάζεται αναθεώρηση. Ακόμη και αν, σε σπάνια περίπτωση, ο κώδικας είναι άψογος, η ανασκόπηση παρέχει μια ευκαιρία για καθοδήγηση και συνεργασία και, ελάχιστα, διαφοροποιεί την κατανόηση του κώδικα στη βάση του κώδικα.

Πότε πρέπει να αναθεωρηθεί

Οι αναθεωρήσεις κώδικα θα πρέπει να πραγματοποιηθούν αφού ολοκληρωθούν επιτυχώς αυτοματοποιημένοι έλεγχοι (δοκιμές, στυλ, άλλα CI), αλλά πριν συγχωνευθεί ο κώδικας στον κεντρικό κατάλογο του αποθετηρίου. Γενικά, δεν πραγματοποιούμε επίσημη αναθεώρηση κώδικα των συνολικών αλλαγών από την τελευταία έκδοση.

Για πολύπλοκες αλλαγές που θα πρέπει να συγχωνευθούν στον κλάδο της κύριας γραμμής ως ενιαία μονάδα αλλά είναι πολύ μεγάλες για να χωρέσουν σε ένα λογικό CR, σκεφτείτε ένα μοντέλο στοίβαξης CR: Δημιουργήστε ένα κύριο χαρακτηριστικό κλάδου / μεγάλο χαρακτηριστικό και έναν αριθμό δευτερευόντων κλάδων (π.χ. / big-feature-api, δοκιμή χαρακτηριστικών / μεγάλων χαρακτηριστικών, κλπ.) που καθένα ενσωματώνει ένα υποσύνολο της λειτουργικότητας και τα οποία εξετάζονται μεμονωμένα σε κώδικα έναντι του κλάδου χαρακτηριστικών / μεγάλων χαρακτηριστικών. Μόλις συγχωνευθούν όλα τα δευτερεύοντα υποκαταστήματα σε χαρακτηριστικό / μεγάλο χαρακτηριστικό, δημιουργήστε ένα CR για τη συγχώνευση του τελευταίου στον κύριο κλάδο.

Προετοιμασία κώδικα για αναθεώρηση

Είναι ευθύνη του συντάκτη να υποβάλει CRs που είναι εύκολο να αναθεωρηθούν, ώστε να μην χάνουν χρόνο και κίνητρο για τους αναθεωρητές:

  • Πεδίο και μέγεθος. Οι αλλαγές πρέπει να έχουν ένα στενό, σαφώς καθορισμένο, αυτοτελές πεδίο κάλυψης το οποίο να καλύπτει εξαντλητικά. Για παράδειγμα, μια αλλαγή μπορεί να εφαρμόσει μια νέα δυνατότητα ή να διορθώσει ένα σφάλμα. Οι βραχύτερες αλλαγές προτιμώνται σε σχέση με τις μακρύτερες. Εάν μια CR κάνει ουσιαστικές αλλαγές σε περισσότερα από 5 αρχεία ή χρειάστηκε περισσότερο από 1-2 ημέρες για να γράψει ή θα χρειαζόταν περισσότερα από 20 λεπτά για να την επανεξετάσει, σκεφτείτε να την χωρίσετε σε πολλαπλές αυτόνομες CR. Για παράδειγμα, ένας προγραμματιστής μπορεί να υποβάλει μια αλλαγή που ορίζει το API για μια νέα δυνατότητα από άποψη διεπαφών και τεκμηρίωσης και μια δεύτερη αλλαγή που προσθέτει εφαρμογές για αυτές τις διεπαφές.
  • Υποβάλετε μόνο πλήρη, αυτο-αναθεωρημένη (από diff), και αυτο-δοκιμασμένο CRs. Για να εξοικονομήσετε χρόνο, δοκιμάστε τις αλλαγές που υποβλήθηκαν (δηλ. Εκτελέστε τη δοκιμαστική σουίτα) και βεβαιωθείτε ότι έχουν περάσει όλες τις κατασκευές καθώς και όλες τις δοκιμές και τους ελέγχους ποιότητας κώδικα, τόσο τοπικά όσο και στους διακομιστές CI, πριν αναθέσουν τους κριτές.
  • Οι αλλαγές που αφορούν τον επαναπροσανατολισμό δεν πρέπει να αλλοιώνουν τη συμπεριφορά. Αντίστροφα, αλλαγές που αλλάζουν συμπεριφορά θα πρέπει να αποφεύγουν τις επαναδιατυπώσεις και τις αλλαγές μορφοποίησης κώδικα. Υπάρχουν πολλοί σοβαροί λόγοι για αυτό:
    - Οι αλλαγές επαναληπτικού ελέγχου συχνά αγγίζουν πολλές γραμμές και αρχεία και συνεπώς θα επανεξεταστούν με λιγότερη προσοχή. Οι ακούσιες αλλαγές συμπεριφοράς μπορούν να διαρρεύσουν στη βάση του κώδικα χωρίς να το παρατηρήσουν κανείς.
    - Μεγάλες αλλαγές στο refactoring σπάσουν την ανάκτηση κερασιών, την επαναχρησιμοποίηση και άλλες μαγικές πηγές ελέγχου πηγών. Είναι πολύ επαχθές να ακυρώσετε μια αλλαγή συμπεριφοράς που εισήχθη ως μέρος μιας δέσμευσης refactoring σε ένα αποθετήριο.
    - Ο ακριβός χρόνος αναθεώρησης του ανθρώπινου δυναμικού πρέπει να δαπανηθεί για τη λογική του προγράμματος και όχι για το στυλ, τη σύνταξη ή τη διαμόρφωση των συζητήσεων. Προτιμούμε να εγκατασταθούν αυτοί με αυτοματοποιημένα εργαλεία όπως Checkstyle, TSLint, Baseline, Prettier κ.λπ.

Εκτελέστε μηνύματα

Το παρακάτω είναι ένα παράδειγμα ενός καλού μηνύματος δέσμευσης σύμφωνα με ένα ευρέως αναφερόμενο πρότυπο:

Κεφαλαιοποίηση, σύντομη περίληψη (80 χαρακτήρων ή λιγότερο)
Λεπτομερέστερο επεξηγηματικό κείμενο, εάν χρειάζεται. Τυλίξτε το σε περίπου 120 χαρακτήρες ή έτσι. Σε μερικά πλαίσια, το πρώτο
η γραμμή αντιμετωπίζεται ως το αντικείμενο ηλεκτρονικού ταχυδρομείου και το υπόλοιπο κείμενο ως σώμα. Η κενή γραμμή που χωρίζει το
η περίληψη από το σώμα είναι κρίσιμη (εκτός αν παραλείψετε το σώμα εξ ολοκλήρου). εργαλεία όπως το rebase μπορεί να μπερδευτούν εάν τρέχετε
τα δύο μαζί.
Γράψτε το μήνυμα δέσμευσης σας στην επιτακτική ανάγκη: "Διορθώστε το σφάλμα" και όχι το "Διορθωμένο σφάλμα" ή "Διορθώνει το σφάλμα". Αυτή η σύμβαση ταιριάζει
με μηνύματα δέσμευσης που δημιουργούνται από εντολές όπως συγχώνευση git και επαναφορά git.
Περαιτέρω παράγραφοι έρχονται μετά από κενές γραμμές.
- Τα σημεία σφαίρας είναι επίσης εντάξει

Προσπαθήστε να περιγράψετε τόσο το τι αλλάζει η δέσμευση όσο και πώς το κάνει:

> ΚΑΛΑ. Μην το κάνετε αυτό.
Κάντε το compile ξανά
> Καλό.
Προσθέστε την εξάρτηση jcsv για να διορθώσετε τη συλλογή IntelliJ

Εύρεση αναθεωρητών

Είναι συνηθισμένο για τον υπεύθυνο να προτείνει έναν ή δύο αναθεωρητές που είναι εξοικειωμένοι με τη βάση κώδικα. Συχνά, ένας από τους αναθεωρητές είναι ο επικεφαλής του έργου ή ένας ανώτερος μηχανικός. Οι ιδιοκτήτες των έργων θα πρέπει να εξετάσουν το ενδεχόμενο να εγγραφούν στα έργα τους προκειμένου να ενημερωθούν για νέες CR. Οι αναθεωρήσεις κώδικα μεταξύ περισσότερων από τριών μερών είναι συχνά μη παραγωγικές ή ακόμα και αντιπαραγωγικές, καθώς διάφοροι αξιολογητές μπορούν να προτείνουν αντιφατικές αλλαγές. Αυτό μπορεί να υποδηλώνει θεμελιώδεις διαφωνίες σχετικά με τη σωστή εφαρμογή και θα πρέπει να επιλυθεί εκτός αναθεώρησης κώδικα σε φόρουμ υψηλότερου εύρους ζώνης, για παράδειγμα προσωπικά ή σε τηλεδιάσκεψη με όλα τα εμπλεκόμενα μέρη.

Εκτέλεση ανασκοπήσεων κώδικα

Μια αναθεώρηση κώδικα είναι ένα σημείο συγχρονισμού μεταξύ διαφορετικών μελών της ομάδας και συνεπώς έχει τη δυνατότητα να εμποδίσει την πρόοδο. Συνεπώς, οι αναθεωρήσεις κώδικα πρέπει να είναι έγκαιρες (με τη σειρά των ωρών, όχι τις ημέρες), και τα μέλη της ομάδας και οι οδηγοί πρέπει να γνωρίζουν τη χρονική δέσμευση και να δώσουν προτεραιότητα στον χρόνο αναθεώρησης αναλόγως. Εάν δεν νομίζετε ότι μπορείτε να ολοκληρώσετε μια αναθεώρηση εγκαίρως, ενημερώστε τον υπεύθυνο για να βρείτε κάποιον άλλο.

Μια αναθεώρηση θα πρέπει να είναι αρκετά διεξοδική ώστε ο αναθεωρητής να μπορεί να εξηγήσει την αλλαγή σε ένα λογικό επίπεδο λεπτομέρειας σε έναν άλλο προγραμματιστή. Αυτό εξασφαλίζει ότι οι λεπτομέρειες της βάσης κώδικα είναι γνωστές σε περισσότερα από ένα άτομα.

Ως κριτικός, είναι δική σας ευθύνη να εφαρμόσετε τα πρότυπα κωδικοποίησης και να διατηρήσετε την μπάρα ποιότητας επάνω. Η ανασκόπηση του κώδικα είναι κάτι περισσότερο από μια τέχνη παρά μια επιστήμη. Ο μόνος τρόπος να το μάθεις είναι να το κάνεις. ένας έμπειρος κριτικός πρέπει να εξετάσει το ενδεχόμενο να θέσει και άλλους λιγότερο έμπειρους κριτικούς σχετικά με τις αλλαγές τους και να τους κάνει πρώτα να τους εξετάσει. Υποθέτοντας ότι ο συγγραφέας ακολούθησε τις παραπάνω οδηγίες (ειδικά όσον αφορά την αυτοεξέταση και την εξασφάλιση της εκτέλεσης του κώδικα), εδώ είναι μια λίστα με τα πράγματα που ένας κριτικός πρέπει να προσέξει σε μια ανασκόπηση κώδικα:

Σκοπός

  • Ο κώδικας αυτός εκπληρώνει το σκοπό του συγγραφέα; Κάθε αλλαγή θα πρέπει να έχει έναν συγκεκριμένο λόγο (νέο χαρακτηριστικό, ανακλαστικό, διορθωτικό bug, κλπ). Ο υποβαλλόμενος κώδικας πράγματι επιτυγχάνει αυτόν τον σκοπό;
  • Κανε ερωτησεις. Οι λειτουργίες και οι κλάσεις πρέπει να υπάρχουν για κάποιο λόγο. Όταν ο λόγος δεν είναι σαφής στον αξιολογητή, αυτό μπορεί να αποτελεί ένδειξη ότι ο κώδικας πρέπει να ξαναγραφεί ή να υποστηριχθεί με σχόλια ή δοκιμές.

Εκτέλεση

  • Σκεφτείτε πώς θα λύσατε το πρόβλημα. Αν είναι διαφορετικό, γιατί είναι αυτό; Ο κώδικας σας χειρίζεται περισσότερα (άκρη) περιπτώσεις; Είναι μικρότερη / ευκολότερη / καθαρότερη / ταχύτερη / ασφαλέστερη αλλά λειτουργικά ισοδύναμη; Υπάρχει κάποιο υποκείμενο μοτίβο που εντοπίσατε και δεν έχει καταγραφεί από τον τρέχοντα κώδικα;
  • Βλέπετε πιθανότητες για χρήσιμες αφαιρέσεις; Ο μερικώς διπλότυπος κώδικας συχνά υποδηλώνει ότι μια πιο αφηρημένη ή γενική λειτουργικότητα μπορεί να εξαχθεί και στη συνέχεια να επαναχρησιμοποιηθεί σε διαφορετικά πλαίσια.
  • Σκεφτείτε σαν έναν αντίπαλο, αλλά να είναι ωραίο για αυτό. Προσπαθήστε να "πιάσετε" τους συγγραφείς να κάνουν συντομεύσεις ή να λείπουν περιπτώσεις αντιμετωπίζοντας προβληματικές διαμορφώσεις / δεδομένα εισόδου που παραβιάζουν τον κώδικα τους.
  • Σκεφτείτε τις βιβλιοθήκες ή τον υπάρχοντα κωδικό προϊόντος. Όταν κάποιος ξαναβάλλει την υπάρχουσα λειτουργικότητα, πιο συχνά απλά δεν είναι επειδή απλά δεν γνωρίζουν ότι υπάρχει ήδη. Μερικές φορές, ο κώδικας ή η λειτουργικότητα διπλασιάζεται με σκοπό, π.χ., προκειμένου να αποφευχθούν εξαρτήσεις. Σε αυτές τις περιπτώσεις, ένα σχόλιο κώδικα μπορεί να διευκρινίσει την πρόθεση. Είναι ήδη η λειτουργικότητα που έχει εισαχθεί από μια υπάρχουσα βιβλιοθήκη;
  • Η αλλαγή ακολουθεί τα τυποποιημένα μοτίβα; Οι καθιερωμένες βάσεις κώδικα συχνά παρουσιάζουν πρότυπα γύρω από συμβάσεις ονομασίας, αποσύνθεση λογικού προγράμματος, ορισμούς τύπων δεδομένων κλπ. Είναι συνήθως επιθυμητό οι αλλαγές να πραγματοποιούνται σύμφωνα με τα υπάρχοντα πρότυπα.
  • Η αλλαγή προσθέτει εξαρτήσεις συμπλήρωσης χρόνου ή χρόνου εκτέλεσης (ειδικά μεταξύ υποέργων); Θέλουμε να διατηρήσουμε τα προϊόντα μας χαλαρά συνδεδεμένα, με όσο το δυνατόν λιγότερες εξαρτήσεις. Οι αλλαγές στις εξαρτήσεις και το σύστημα κατασκευής θα πρέπει να εξεταστούν σε μεγάλο βαθμό.

Ευανάγνωστο και στιλ

  • Σκεφτείτε την εμπειρία ανάγνωσης. Έχετε κατανοήσει τις έννοιες σε εύλογο χρονικό διάστημα; Ήταν η λογική ροή και ήταν μεταβλητές και ονόματα μεθόδων εύκολο να ακολουθήσει; Ήταν σε θέση να παρακολουθείτε μέσω πολλαπλών αρχείων ή λειτουργιών; Μήπως ανατράπηκε από ασυνεπή ονομασία;
  • Συμμορφώνεται ο κώδικας με τις οδηγίες κωδικοποίησης και το στυλ κώδικα; Είναι ο κώδικας συνεπής με το έργο όσον αφορά το στυλ, τις συμβάσεις API κ.λπ .; Όπως προαναφέρθηκε, προτιμούμε να διευθετήσουμε τις συζητήσεις στυλ με τα αυτοματοποιημένα εργαλεία.
  • Ο κώδικας αυτός έχει TODOs; TODOs απλά συσσωρεύονται σε κώδικα, και να γίνει ξεπερασμένη με την πάροδο του χρόνου. Υποβάλετε το εισιτήριο στο GitHub Issues ή JIRA και επισυνάψτε τον αριθμό έκδοσης στο TODO. Η προτεινόμενη αλλαγή κώδικα δεν πρέπει να περιέχει κώδικα σχολιασμένο.

Διατηρησιμότητα

  • Διαβάστε τις δοκιμές. Εάν δεν υπάρχουν δοκιμές και πρέπει να υπάρχει, ζητήστε από τον συγγραφέα να γράψει μερικά. Πραγματικά αβέβαια χαρακτηριστικά είναι σπάνια, ενώ δυστυχώς είναι κοινά μη δοκιμασμένα υλοποιήσεις χαρακτηριστικών. Ελέγξτε τα ίδια τα τεστ: καλύπτουν ενδιαφέρουσες περιπτώσεις; Είναι ευανάγνωστα; Μειώνει η συνολική δοκιμαστική κάλυψη CR; Σκεφτείτε τρόπους που θα μπορούσαν να σπάσουν ο κώδικας. Τα πρότυπα στυλ για τις δοκιμές είναι συχνά διαφορετικά από τον βασικό κώδικα, αλλά εξακολουθούν να είναι σημαντικά.
  • Αυτή η CR εισάγει τον κίνδυνο να σπάσει τον κώδικα δοκιμής, τις δοκιμασίες στοίβαξης ή τις δοκιμές ολοκλήρωσης; Αυτά συχνά δεν ελέγχονται ως μέρος των ελέγχων πριν από τη συνένωση / συγχώνευση, αλλά η πτώση τους είναι οδυνηρή για όλους. Συγκεκριμένα πράγματα που πρέπει να αναζητήσετε είναι: η αφαίρεση των βοηθητικών προγραμμάτων ή των τρόπων δοκιμής, οι αλλαγές στη διαμόρφωση και οι αλλαγές στη διάταξη / δομή του τεχνητού.
  • Μήπως αυτή η αλλαγή σπάσει την συμβατότητα προς τα πίσω; Εάν ναι, είναι σωστό να συγχωνεύσετε την αλλαγή σε αυτό το σημείο ή θα έπρεπε να προωθηθεί αργότερα; Τα διαλείμματα μπορούν να περιλαμβάνουν αλλαγές βάσεων δεδομένων ή σχημάτων, δημόσιες αλλαγές API, αλλαγές ροής εργασίας χρήστη κ.λπ.
  • Ο κώδικας αυτός χρειάζεται δοκιμές ολοκλήρωσης; Μερικές φορές, ο κώδικας δεν μπορεί να δοκιμαστεί επαρκώς μόνο με δοκιμές μονάδων, ειδικά εάν ο κώδικας αλληλεπιδρά με εξωτερικά συστήματα ή διαμόρφωση.
  • Αφήστε σχόλια σχετικά με την τεκμηρίωση σε επίπεδο κώδικα, τα σχόλια και τα μηνύματα δέσμευσης. Τα περιττά σχόλια γεμίζουν τον κώδικα και τα μηνύματα απόσχισης δεσμεύουν τους μελλοντικούς συνεισφέροντες. Αυτό δεν ισχύει πάντοτε, αλλά τα σχόλια ποιότητας και τα μηνύματα δέσμευσης θα πληρώσουν για τον εαυτό τους κάτω από τη γραμμή. (Σκεφτείτε έναν χρόνο που έχετε δει ένα εξαιρετικό ή πραγματικά τρομερό μήνυμα ή σχόλιο).
  • Έχει ενημερωθεί η εξωτερική τεκμηρίωση; Εάν το έργο σας διατηρεί ένα README, CHANGELOG ή άλλη τεκμηρίωση, ενημερώθηκε για να αντικατοπτρίζει τις αλλαγές; Η παρωχημένη τεκμηρίωση μπορεί να είναι πιο συγκεχυμένη από καμία και θα είναι πιο δαπανηρή η επίλυσή της στο μέλλον παρά η ενημέρωσή της τώρα.

Μην ξεχάσετε να επαινείτε συνοπτικός / αναγνώσιμος / αποτελεσματικός / κομψός κώδικας. Αντίθετα, η μείωση ή η αποδοχή μιας CR δεν είναι αγενής. Εάν η αλλαγή είναι περιττή ή άσχετη, απορρίψτε την με μια εξήγηση. Αν το θεωρείτε απαράδεκτο λόγω ενός ή περισσότερων μοιραίων ελαττωμάτων, απορρίψτε το, και πάλι με μια εξήγηση. Μερικές φορές το σωστό αποτέλεσμα μιας CR είναι "να το κάνουμε αυτό εντελώς διαφορετικό" ή ακόμα και "να μην το κάνουμε καθόλου".

Να είστε σεβασμός στους κριτές. Ενώ η αντιπαράθεση είναι χρήσιμη, δεν είναι το χαρακτηριστικό σας και δεν μπορείτε να κάνετε όλες τις αποφάσεις. Εάν δεν μπορείτε να καταλήξετε σε συμφωνία με τον εξεταστή σας με τον κώδικα ως έχει, μεταβείτε στην επικοινωνία σε πραγματικό χρόνο ή αναζητήστε μια τρίτη γνώμη.

Ασφάλεια

Βεβαιωθείτε ότι τα τελικά σημεία API εκτελούν την κατάλληλη εξουσιοδότηση και έλεγχο ταυτότητας σύμφωνα με την υπόλοιπη βάση κώδικα. Ελέγξτε για άλλες συνήθεις αδυναμίες, π.χ. αδύναμη ρύθμιση παραμέτρων, εισαγωγή κακόβουλου χρήστη, συμβάντα που δεν έχουν καταγραφεί, κλπ. Σε περίπτωση αμφιβολίας, συμβουλευτείτε το CR σε έναν ειδικό ασφαλείας εφαρμογών.

Σχόλια: Συνοπτική, φιλική, ενεργητική

Οι αναθεωρήσεις πρέπει να είναι συνοπτικές και γραπτές σε ουδέτερη γλώσσα. Κρίστε τον κώδικα και όχι τον συγγραφέα. Όταν κάτι είναι ασαφές, ζητήστε διευκρίνιση και όχι υποθέστε άγνοια. Αποφύγετε τις κτητικές αντωνυμίες, ιδίως σε συνδυασμό με τις αξιολογήσεις: "ο κώδικας μου λειτούργησε πριν από την αλλαγή σας", "η μέθοδος σας έχει ένα σφάλμα" κλπ. Αποφύγετε απόλυτες κρίσεις: "αυτό δεν μπορεί ποτέ να λειτουργήσει", "το αποτέλεσμα είναι πάντα λάθος".

Προσπαθήστε να διαφοροποιήσετε μεταξύ των προτάσεων (π.χ. "Πρόταση: μέθοδο εξαγωγής για τη βελτίωση της αναγνωσιμότητας"), απαιτούμενες αλλαγές (π.χ. "Add @Override") και σημεία που χρειάζονται συζήτηση ή διευκρίνιση (π.χ. οπότε, προσθέστε ένα σχόλιο που εξηγεί τη λογική. "). Εξετάστε το ενδεχόμενο να δώσετε συνδέσμους ή δείκτες για να εξηγήσετε σε βάθος ένα πρόβλημα.

Όταν τελειώσετε με μια αναθεώρηση κώδικα, υποδείξτε σε ποιο βαθμό αναμένετε από τον συγγραφέα να απαντήσει στα σχόλιά σας και αν θα θέλατε να επανεξετάσετε το CR μετά την εφαρμογή των αλλαγών (π.χ. "Αισθανθείτε ελεύθερος να συγχωνευθεί μετά την απάντηση στις λίγες μικρές προτάσεις "εναντίον" Παρακαλώ εξετάστε τις προτάσεις μου και ενημερώστε μου πότε μπορώ να κάνω μια άλλη ματιά. ").

Απαντώντας σε κριτικές

Μέρος του σκοπού της αναθεώρησης κώδικα είναι να βελτιωθεί το αίτημα αλλαγής του συντάκτη. κατά συνέπεια, να μην προσβάλλονται από τις προτάσεις του κριτικού σας και να τους παίρνετε σοβαρά ακόμα και αν δεν συμφωνείτε. Απάντηση σε κάθε σχόλιο, ακόμα και αν είναι απλό "ACK" ή "έτοιμο". Εξηγήστε γιατί κάνατε ορισμένες αποφάσεις, γιατί υπάρχει κάποια λειτουργία κλπ. Εάν δεν μπορείτε να καταλήξετε σε συμφωνία με τον κριτή, ή να αναζητήσετε μια εξωτερική γνώμη.

Οι διορθώσεις πρέπει να ωθηθούν στον ίδιο κλάδο, αλλά σε ξεχωριστή δέσμευση. Το Squashing δεσμεύεται κατά τη διάρκεια της διαδικασίας επανεξέτασης καθιστά δύσκολο για τον αξιολογητή να παρακολουθήσει τις αλλαγές.

Οι διαφορετικές ομάδες έχουν διαφορετικές πολιτικές συγχώνευσης: ορισμένες ομάδες επιτρέπουν τη συγχώνευση μόνο των κατόχων έργων, ενώ άλλες ομάδες επιτρέπουν στο συνεργάτη να συγχωνεύσει μετά από μια θετική αναθεώρηση κώδικα.

Κριτικές προσωπικού κώδικα

Για την πλειοψηφία των αναθεωρήσεων κώδικα, τα ασύγχρονα εργαλεία που βασίζονται σε diff, όπως το Reviewable, το Gerrit ή το GitHub, είναι μια εξαιρετική επιλογή. Οι πολύπλοκες αλλαγές ή οι αναθεωρήσεις μεταξύ των μερών με πολύ διαφορετική εμπειρογνωμοσύνη ή εμπειρία μπορεί να είναι πιο αποτελεσματικές όταν πραγματοποιούνται αυτοπροσώπως είτε μπροστά από την ίδια οθόνη είτε με προβολέα ή εξ αποστάσεως μέσω εργαλείων VTC ή μεριδίου οθόνης.

Παραδείγματα

Στα ακόλουθα παραδείγματα, τα προτεινόμενα σχόλια ανασκόπησης υποδεικνύονται με // R: ... σχόλια στα μπλοκ κώδικα.

Ακατάλληλη ονομασία

class MyClass {
  ιδιωτικό int countTotalPageVisits; // R: ονομάστε μεταβλητές με συνέπεια
  ιδιωτικό int uniqueUsersCount;
}}

Αντιφατικές υπογραφές μεθόδων

interface MyInterface {
  / ** Επιστρέφει {@ Προαιρετικό # κενό} αν το s δεν μπορεί να εξαχθεί. * /
  δημόσιο προαιρετικό  extractString (String s);
  / ** Επιστρέφει null αν {@code s} δεν μπορεί να ξαναγραφεί. * /
  // R: θα πρέπει να εναρμονίσει τις τιμές επιστροφής: χρησιμοποιήστε και το προαιρετικό <> εδώ
  δημόσιο String rewriteString (String s)?
}}

Χρήση βιβλιοθήκης

// R: αφαιρέστε και αντικαταστήστε από το MapJoiner της Guava
Συμβολοσειρά joinAndConcatenate (Map  map, String keyValueSeparator, String keySeparator)?

Προσωπικό γούστο

int dayCount; // R: nit: συνήθως προτιμώ numFoo πάνω fooCount? αλλά θα πρέπει να το διατηρήσουμε συνεπείς σε αυτό το έργο

Σφάλματα

// R: Αυτό εκτελεί numIterations + 1 επαναλήψεις, είναι αυτό σκόπιμο;
// Αν είναι, σκεφτείτε να αλλάξετε τη σημασιολογία των αριθμητικών;
για το (int i = 0, i <= numIterations, ++ i) {
  ...
}}

Αρχιτεκτονικές ανησυχίες

otherService.call (); // R: Νομίζω ότι θα πρέπει να αποφύγουμε την εξάρτηση από το OtherService. Μπορούμε να το συζητήσουμε αυτοπροσώπως;

Συγγραφείς

Robert F (GitHub / Twitter)