Przedwczoraj Radek Maziarka opublikował draft (?) artykułu “Wysoka jakość kodu bez code review” (dostępny tutaj). W artykule Radek stawia tezę “w przeważającej liczbie przypadków PR Review jest zabójcze dla szybkości dostarczania“. Czy zawsze tak jest bądź musi być czy to jednostkowe przypadki? Ponieważ Radek w swoim artykule wypisał listę powodów, dla których warto mieć review jak i również dlaczego nie warto, ja skupię się analizie warunków, w których dochodzi do problemów z review i dlaczego uważam, że “w przeważającej liczbie przypadków” jest swego rodzaju nadużyciem.

W swoich rozważaniach uwzględnię:

• wielkość zespołu
• doświadczenie członków zespołu
• rozproszenie geograficzne
• rozmiar zadań i ich dostarczanie

Na początku stawiam tezę, że problem z PR review pojawia się głównie w zespołach, gdzie pojedyncze zadania są bardzo duże i przez to rozciągnięte w czasie. Jakie problemy to generuje? Efektem finalnym jest dużo kodu do przejrzenia, konflikty ze zmieniającym się bazowym branchem (czy to development czy inny branch, gdzie pozostali członkowie zespołu mogli już dużo zmienić). W swoim zespole stoję właśnie przed takim wyzwaniem i w naszym przypadku prawdopodobnie rozwiązaniem będzie stosowanie “feature flags” (więcej o tym mechaniźmie tutaj) tak, żeby można było deployować jeszcze nie kompletne rozwiązanie bez wpływu na działające rozwiązanie.

Przykładem takiego dużego zadania może być refactoring istotnego serwisu w rozproszonym monolicie (dużo warstw, a także dużo klientów, które trzeba dostosować do zmian). Zatem sam sposób rozdziału pracy i podejście do kompatybilności mają znaczący wpływ na szybkość dostarczania.

Dodatkowym problemem jest przetestowanie takiej dużej paczki zmian. Z reguły w zespołach, w których pojawiają się takie duże zadania, proces testowania działa słabo albo nie działa wcale (braki w testach jednostkowych lub ich całkowity brak, testy wyłącznie manualne, brak testów regresji i integracyjnych).

W związku z powyższym, pierwszym problemem jaki należy rozważyć jest sposób dostarczania zmian jako całościowy proces. Począwszy od rozmiaru aż do zależności, tak żeby pojedynczy członek zespołu był wstanie podjąć kolejne zadanie nim koledzy dokonają code review jego PR.

Przyjrzyjmy się teraz wielkości zespołu – czy on ma jakiejkolwiek znaczenie? Zdecydowanie nie. Pracowałem w bardzo małych jak i bardzo dużych zespołach, zarówno zlokalizowanych w jednym miejscu jak i rozproszonych, z wieloma programistami o różnym doświadczeniu i głównym czynnikiem jaki wpływał na szybkość przejrzenia kodu była kultura pracy. W sytuacji, w której review jest elementem kultury, wymuszonym przez zespół na każdym jego członku, każdemu zaczyna zależeć na szybkim przejrzeniu zmian kolegi (szybkie != byle jakie). Na drugim końcu są zespoły, których członkowie skupiają się na dowożeniu własnego kodu bez patrzenia na to co robią koledzy. W takiej kulturze pracy, zawsze powstaną opóźnienia i kolejki, niezależnie od tego czy PR będzie mały czy duży.

Dodatkowym aspektem jest niechęć i brak możliwości koncentracji w dłuższym czasie na kodzie. Jeśli mamy PR, który zmienia 300 plików, wiadomym jest, że chociażby jego pobieżne sprawdzenie zajmie ponad godzinę (a prawdopodobnie kilka), a efektem końcowym będzie niewiele komentarzy, a na końcu “looks good”. Natomiast w przypadku małego fragmentu kodu, dużo prościej jest się skupić na tym co i jak danych kod robi, zatem łatwiej jest go przeanalizować.

Spójrzmy jeszcze na rozproszenie geograficzne – w przypadku code review, istotne może być zasięgnięcie opinii kogoś bardziej doświadczonego w danym obszarze. Jeśli zespół pracuje w wielu strefach czasowych, zrobienie peer review na zasadzie zdzwonienia się, wymaga dopasowania po obu stronach, a to może być killer, jeśli część zespołu jest np w Indiach, część w Polsce, a część na zachodnim wybrzeżu. W takim wypadku “tradycyjne” PR review sprawdza się znacznie lepiej, gdyż jest w pełni asynchroniczne.

Spójrzmy jeszcze na dodatkowy aspekt w kontekście review – monorepo vs repo per projekt. Dlaczego ma to wpływ na tempo dostarczania? Przy monorepo, tworząc review mamy 1 branch i jeden PR, przy repo per projekt mogą pojawić się zależności wpływające na to co i kiedy możemy zmerge’ować, a następnie zrelease’ować. Przykładowo nowa funkcjonalność biznesowa (np. wyszukiwanie po dodatkowym filtrze) nie może zostać udostępniona klientowi, jeśli backend aplikacji tego nie wspiera, zatem najpierw konieczne jest wydanie backendu a następnie frontendu.

Podsumowanie

W aspekcie PR/code review, kluczowe pod kątem dostarczania są 2 czynniki:

• rozmiar zadań
• ludzie i ich kultura pracy

Jeśli nie zadbamy o dostatecznie małe zadania, skończymy z dużymi zadaniami, dużą ilością zmian w kodzie, która jest dla ludzi zniechęcająca. Zatem dbajmy o odpowiednie planowanie pracy, żeby utrzymać wysoką kulturę pracy.

Dla mnie PR review to hit i projektowe “must have” ze względu na:

• dzielenie się zmianami między zespołami
• możliwość innego spojrzenia na rozwiązanie
• walidację czy kod robi to co powinien i tak jak powinien

I nawet, jeśli trochę spowalnia dostarczenie, to jest tego warte w warunkach, w których pracowałem i obecnie pracuję.