Code review jest bardzo ważne. Podnosi jakość kodu, poprawia komunikację i rozpowszechnia wiedzę o projekcie w zespole. Są to tylko nieliczne wartości dodane, a jest ich znacznie więcej. Dobry proces sprawdzania kodu przed commit’em, ma też wartość edukacyjną. Osoba sprawdzająca kod (reviewer) z zasady powinna posiadać co najmniej takie samo albo i większe doświadczenie niż osoba, która ten kod udostępnia do weryfikacji. Co za tym idzie, istnieje prawdopodobieństwo, że reviewer ma większą wiedzę i może się nią podzielić z autorem kodu podczas code review.
Z pozoru wydaje się, że jest to trywialny proces. Programista piszę kod, udostępnia go innemu programiście do sprawdzenia, ten nanosi swoje uwagi, poprawka kodu i commit! Czy na pewno jest to takie proste? Czy zastanawiałeś(aś) się kiedyś, dlaczego code review w Twojej firmie jest traktowane po macoszemu? Dlaczego trwa długo? Albo czemu pomimo praktykowania tego procesu jakość kodu nie wzrasta a ilość bugów nie maleje?
Oto 5 często popełnianych błędów przez programistów podczas procesu code review. Zobacz jak możesz znacząco skrócić czas całego procesu i jak uchronić się przed bezustannym poprawianiem tego samego kodu.
1. Brak przedstawienia problemu, wymagań biznesowych
Niewiele osób dostrzega ten problem podczas code review. Często cały proces weryfikacji zmian polega na przejrzeniu kodu i jeśli nie ma tam nic niepokojącego, to reviewer daje zielone światło autorowi. Niestety często zdarza się tak, że po, mimo iż kod nie posiada rażących błędów, które są widoczne na pierwszy rzut oka, to nie zawsze jest w stanie realizować postawione przed nim wymagania biznesowe. Przedstawienie problemu, który ma być rozwiązany poprzez konkretną implementację, jest kluczowe w tym procesie. Reviewer dzięki temu ma szerszy obraz tego co powinno zostać zaimplementowane przez autora. Krótkie, nawet kilku zdaniowe, wprowadzenie podczas rozmowy, podlinkowanie numeru ticketu lub opisu wymagania biznesowego w komentarzu commit’a może znacząco ułatwić cały proces. Reviewer nie musi się zastanawiać „co autor miał na myśli” pisząc ten kod. Może też zasugerować inne, lepsze rozwiązanie, które będzie lepiej pasowało do realizacji danej funkcjonalności lub poprawi bug w lepszy, „trwalszy” sposób.
2. Brak podziału na sprawdzenie jakości kodu i pokrycie wymagań biznesowych
Skoro już jesteśmy przy wymaganiach biznesowych, to warto też wspomnieć, że code review powinno być podzielone na dwie części. Pierwsza z nich to sprawdzenie, czy dany kod będzie realizował założone wymagania biznesowe Druga, czy jest poprawny i nie ma w nim po prostu zwyczajnych bugów. Ten punkt faktycznie pokrywa się nie co z poprzednim, ale tutaj chciałbym zwrócić Twoją uwagę na to, że poprawny kod pod względem technicznym nie zawsze będzie poprawny pod względem biznesowym. Pewnie nie raz zdarzyło Ci się dostać tego samego taska ponownie, z komentarzem „coś nie działa jak powinno” albo „to nie tak miało wyglądać”. Pomimo że Twój commit przeszedł przez code review bez problemów (albo wszystkie zostały poprawione), to musisz ponownie pochylić się nad tym samym kodem.
Podział code review na dwie części znacząco zmniejsza ryzyko powrotu tego samego taska do sprintu. Taki proces może być przeprowadzany przez jedną osobę w charakterze reviewer’a, ale najlepiej jak by były to dwie inne osoby. Jedna z nich z większą wiedzą techniczną a druga ze znajomością wymagań biznesowych. Zatwierdzenie Twoich zmian przez te dwie osoby podniesie nie tylko jakość kodu, ale też i zgodność z wymaganiami.
3. Niepoprawne review testów
Temat rzeka. Kiedyś, gdy byłem na początku swojej „kariery programistycznej”, pierwszego dnia w nowej firmie wykonałem checkout kodu i puściłem build. Po chwili oczywiście build się wywalił z powodu testów. Zapytałem siedzącego obok mnie kolegę, o co chodzi i dlaczego testy nie przechodzą? Odpowiedź była zaskakująca, odparł „Ah tak, musisz dodać -DskipTests bo te testy i tak nie działają” – dla ciekawskich polecenie „-DskipTests” w Maven pomija wykonywanie testów podczas buildu. Tak, to był ciężki okres. Oprogramowanie to było bardzo niskiej jakości.
Wracając do tematu. Wiele razy widziałem, i Ty pewnie też, commit’y nowych funkcjonalności lub fixy istniejących bez dopisywania nowych albo poprawy istniejących testów. Przejście przez code review takiej zmiany wydaje się wręcz niedorzeczne. Dlaczego pisząc kod od nowa nie jest on pokryty testami? Nie da się tego przetestować? Nie umiesz? Nie chce Ci się, bo wiesz, że i tak przejdzie przez code review? To może oznaczać tylko tyle że, albo źle napisałeś(aś) daną funkcjonalność, albo że proces code review w Twojej firmie istnieje tylko po to, żeby dział HR mógł się chwalić na rekrutacjach, że macie coś takiego.
Kolejnym problemem z code review testów jest właśnie ich code review 😊 Wiem, masło maślane. Czasami zdarza się, że testy są traktowane nie co mniej rygorystycznie niż kod produkcyjny. Co oczywiście jest kompletnie bezpodstawne, ponieważ te testy uruchamiają i testują kod produkcyjny! Dlaczego więc mają być napisane gorzej? Niepoprawnie napisany test, pomimo tego, że przechodzi, to nie testuje danej metody czy funkcjonalności w odpowiedni sposób. Stwarza to pole do występowania bugów w kodzie, a co za tym idzie kolejne taski, które trzeba szybko zafixować w najnowszym releasie.
Idźmy dalej! Niepoprawnie napisany test nie oznacza zawsze, że nie pokrywa w dostatecznym stopniu danej metody czy funkcjonalności. Może też oznaczać, że dany test jest napisany zbyt gorliwie. Nie daje możliwości refetoringu testowanego kodu i wytwarza zjawisko „betonowania” kodu – braku możliwości refactoryzacji kodu bez gruntownych zmian w testach. Takie przypadki powinny być wyłapywane podczas code review, aby projekt był rozwijalny. W innym przypadku często zdarza się, że mała zmiana lub refactoring powoduje lawinowe zmiany w dużej liczbie testów, co zwiększa wydatki z budżetu projektu.
4. Za dużo zmian w jednej sesji code review
Statystyczny człowiek – jak by nie było programiści też się do tej grupy zaliczają – przestaje się skupiać na wykonywanej czynności po około 15 minutach. Później umysł zmierza w kierunku innych myśli typu „Co będzie dzisiaj na obiad?” albo „Chyba muszę do łazienki”. Co za tym idzie, zbyt długie code review traci na jakości. Rozproszony umysł reviewera nie jest w stanie działać na pełnych obrotach przez trwające godzinę albo i więcej przeglądanie zmian w kodzie i jednoczesne śledzenie czy wszystkie są poprawne. Czas poświęcony na sprawdzenie jednej linii kodu skraca się diametralnie, jeśli code review trwa za długo. Reviewer dostaje „magicznych zdolności” sprawdzania nawet dziesiątek linii kodu w ciągu sekundy!
Ok, czasami nie da się zrobić code review w mniej niż 15 min, ponieważ dana funkcjonalność jest na tyle skomplikowana lub rozbudowana. Czy to przypadkiem nie oznacza, że ta funkcjonalność powinna być podzielona na bardziej atomowe części?
5. Niepoprawne przygotowanie kodu do review
Problemem jest też brak odpowiedniego przygotowania zmian przed udostępnieniem do code review. Kod napisany niespójnie z ogólnie przyjętymi praktykami w zespole lub projekcie staje się mniej czytelny. Warto zwrócić uwagę na formatowanie kodu oraz jego zgodność z zasadami „clean code”. Trzymanie się tych utartych ścieżek pozwoli Ci na zmniejszenie ilości poprawek zgłaszanych przez reviewerów do Twoich zmian.
Dobrą praktyką jest korzystanie z narzędzi do statycznej analizy kodu takich jak SonarQube czy CheckStyle. Dzięki tym i innym podobnym, narzędziom będziesz w stanie znacznie szybciej wyłapać miejsca w kodzie, które powinny zostać poprawione.
Zakończenie
Ta lista jest moim subiektywnym przemyśleniem na ten temat. Pewnie masz też swoje, więc zachęcam Cię do podzielenia się nimi w komentarzach.
Standardowo zachęcam też do udostępnienia tego postu w social mediach oraz polubienia fanpage Ara Software na Facebooku!
Przydatne linki: