Code Review - marcinogo/robot GitHub Wiki
-
Poprawne budowanie, jeśli projekt nie buduje się na Jenkinsie nawet nie sprawdzamy projektu
-
Czy projekt buduje się bez błędów i ostrzeżeń
-
Raporty Checkstyle, Sonara, Spotbugs, Jacoco itd. - czy błędy i ostrzeżenia wygenerowane w raporcie można na pewno zignorować
-
Czy jest dokumentacja do publicznych API
-
Czy dokumentacja jest napisana odpowiednio
-
Czy szczegółowo tłumaczy funkcjonalność i działanie klas i metod.
-
Czy dokumentacja zawiera odpowiednie tagi i linki (@link, @author itp.)
Sprawdzamy czystość kodu - wg zasad z Clean Code wujka, czyli:
-
modyfikatory dostępu
-
obecność martwego kodu
-
literówki, poprawny angielski
-
wszelkie code smells
-
Poprawność testów:
-
Czy pokrycie jest odpowiednie
-
Czy testy są dobrze nazwane
-
Czy pisane w konwencji GWT
-
Czy wybrane testy posiadają Data Provider’y
-
Czy rzeczywiście sprawdzają to co powinny
-
Czy testują wszystkie przypadki nie tylko szczęśliwą ścieżkę
Robiąc recenzję należy szczegółowo opisać rzeczy do poprawy oraz, gdy recenzja odrzuca “pull request” podać wszelkie powody dlaczego, tak aby z napisanych komentarzy można było jednoznacznie określić co jest do poprawy.
Recenzja kodu to nie 30 min tylko parę godzin, róbcie je dokładnie i szczegółowo. Fajny artykuł dotyczący dobrych praktyk podczas recenzowania kodu:
W lokalnym repozytorium odpalamy komendę:
mvn clean verify site
Po budowie projektu sprawdzamy raporty znajdujące się w folderze target/site. Polecam odpalić je w przeglądarce bo mamy proste i przejrzyste GUI. Jeśli projekt ma kilka modułów to dla każdego modułu będą niezależne raporty!
Po zrobieniu zadania wystawiamy "pull request" i bierzemy się za recenzowanie innego. Pamiętajcie, każde zadanie musi być zatwierdzone przez co najmniej 2 recenzentów. Śmiało można też prosić wybrane osoby o recenzje, jeśli mają na to czas. Zadanie, które przejdzie recenzje powinno znaleźć się w kolumnie “do scalenia” na tablicy zadań. Potem jest scalane do deva. Jeśli dev zostanie scalony z masterem to zadania lądują w kolumnie “wypchnięte i święte” i można je uznać za zamknięte.