Jak robić dobre code review?

Code review, to jeden z ważniejszych etapów wytwarzania oprogramowania. Zauważyłem ostatnio, ze temat jest traktowany trochę po macoszemu. Jak robić dobre code review? Postaram się na to pytanie odpowiedzieć jak najlepiej a przynajmniej dać wskazówki.

To, jak sprawnie tworzone jest oprogramowanie zależy od wielu czynników. Między innymi jest to zgranie zespołu (czy nie ma konfliktów) czy jak dobrze „wyceniane” są zadania. Zgrany zespół pozwala na swobodną wymianę opinii. Łatwiej wtedy jest powiedzieć, że kod jest do dupy. Nikt nie będzie miał o to pretensji, a może zmobilizować do napisania lepszego, czytelniejszego kodu.

Czym jest code review?

Przede wszystkim jest to dyskusja. Dyskusja pomiędzy autorem a innymi programistami. Nikt nie powinien bać się tutaj wyrazić swojej opinii czy zadać pytania. Efektem końcowym code review powinien być kod, który nie jest podatny na błędy. Zaś osoby biorące udział w tym procesie powinny wyjść bogatsze w wiedzę. Czasami code review to sztuka podejmowania trudnych kompromisów, a czasem marnowaniem czasu na „filozofowanie”.

Większość się pewnie zgodzi ze stwierdzeniem, że kod można podzielić na trzy kategorie:

Działa ale jest ciężki w utrzymaniu. Oznacza to, że każda większa zmiana w kodzie może wpływać negatywnie na istniejącą funkcjonalność.
Działa i dobrze się go czyta. Jest to idealna sytuacja, którą chciała by mieć każda firma. Wymaga to jednak dużej dyscypliny w zespołach czy programistach.
Kombinacja dwóch powyższych punktów. Prawdopodobnie najczęstsza sytuacja w kodzie. Zdarzają się miejsca w kodzie które można umieścić w pierwszym jak i drugim punkcie.

Także na etapie code review ustalane jest w której z powyższych kategorii będzie znajdował się kod. U nas w zespole merge jest robiony gdy kod jest zaakceptowany przez wszystkich.

Jak zatem robić dobre code review?

Na pewno nie ma tutaj złotego środka, który rozwiąże wszystkie problemy. Sprawę na pewno ułatwią firmowe standardy (guidlines) instruujące jak pisać kod. W przypadku frontendu cześć rzeczy można zrobić takimi narzędziami jak prettier czy rożnego rodzaju linterami. Na nasze nieszczęście tego typu dodatki odpowiadają za wizualną część kodu. Nie zastąpią ludzkiego pierwiastka.

Co można zrobić oddając kod do code review?

Zastanów się czy kod dobrze się czyta, czy można coś napisać lepiej czy krócej. Czy w dobrym miejscu znajduje się plik i czy ma to jakieś uzasadnienie? A może coś da się wydzielić lub zrobić używalne?
Nie wrzucaj wszystkiego jako jeden, wielki commit. W miarę możliwości dziel kod na mniejsze porcje. Dużo łatwiej analizuje się mniejszą ilość zmian. Po za tym, można prześledzić proces zmian.
Dodaj do swojego kodu komentarze opisujące dane rozwiązanie. Dzięki temu osoba po drugiej stronie będzie mogła zrozumieć twoje intencje i powód (kontekst) zmian. Może się okazać, że zaproponowane rozwiązanie jest optymalne w tym konkretnym przypadku.
W przypadku wprowadzania zmian w kodzie nie commituj ich jako cr fixes! To nic nie mówi. Opisuj faktyczne zmiany w kodzie. Idealnie tutaj sprawdzi się conventional commits (pro tip: w przypadku wprowadzania zmian w kodzie będziesz chciał poprzedzić swój commit message przez refactor:).

Staraj się niedociągać kodu z innych branchy. Wprowadza to duży szum informacyjny. Merge kodu powinien być ostatnią czynnością jaką robisz.

Co zrobić gdy jesteś proszony o code review?

1. W miarę możliwości zrób to szybko

Code review to tylko część całego procesu wytwarzania oprogramowania. Taki kod musi być jeszcze przetestowany przez QA. Im dłużej zwlekasz z wykonaniem tego zadania tym bardziej blokujesz proces. Oczywiście jeżeli ilość kodu jest większa, wiadomym jest, że nie da się tego sprawdzić i „klepnąć” w pięć minut.

2. Czy kod dobrze wygląda?

Czy do funkcji czy też komponentu nie jest przekazywanych za dużo argumentów? Czy dany kawałek kodu robi to co trzeba? Czy znajduje się w odpowiednim miejscu? A może jakiś warunek jest zbyt skomplikowany i należałoby coś uprościć? A może nazwy zmiennych lub funkcji nie są najlepiej nazwane? Wszystko to (i pewnie kilka innych rzeczy) wpływa na jakość i utrzymanie kodu.

3. Uruchom kod lokalnie

Bardzo dobrym nawykiem jest sprawdzenie członka zespołu czy poprawnie zaimplementował funkcjonalność lub poprawił błąd. Czy wszystko działa jak należy, czy podstawowe przypadki są pokryte. U nas w firmie jest tak, że przed oddaniem kodu do testów inna osoba z zespołu (nie QA) musi sprawdzić czy wszystko jest poprawnie zaimplementowane. Dzięki temu zmniejszamy szansę do pojawienia się błędu w późniejszym etapie.

4. Testy!

Dobrze sprawdzić czy kod został pokryty testami (są od tego nawet automaty). Wiem, że z testami jest różnie i zależy to od wielu czynników. Warto zwrócić na to uwagę. Pokrycie kodu testami, nawet tego ciężkiego w utrzymaniu, pozwoli w części ograniczyć późniejsze problemy. Jeżeli są testy, to warto zwrócić uwagę co w nich się znajduje i czy ma to sens. Czy snapshoty komponentów reactowych (czy też stylów css) są wystarczające?

5. Stwórz dedykowane spotkanie by omówić kod lub problemy

W moim aktualnym zespole nad wyraz dobrze sprawdzają się meetingi na temat kodu. Każdy z nas pokazuje co zrobił i dlaczego. Następnie każdy zadaje pytania, które przekształcają się w ciekawe dyskusje. Wcześniej pisaliśmy sobie dużo komentarzy, ale nie sprawdzały się one dobrze przy bardziej złożonych problemach. Takie spotkanie to dobry moment na live coding (czy też pair coding) w którym rozwiązujemy razem dany problem.

6. Bądź uprzejmy…

O tym chyba nie trzeba wspominać. Czasami spotykam się z sytuacjami w których niektórzy lubią dodać uszczypliwe komentarze w kodzie dla zasady lub mają z kim (np. w zespole) chłodniejsze relacje. Pamiętaj też, że jeżeli zobaczysz jakieś ciekawe rozwiązanie – nie bój się pochwalić.

Na tego typu rzeczy zwracam uwagę gdy jestem po jednej lub drugiej stronie code review. A jak to wygląda w waszych zespołach? Robicie coś inaczej?