Temat: Code Review

Hej,


Wprowadzamy w naszej firmie code review.
Od czego zacząć?
Macie jakieś ciekawe doświadczenia, rady?
Czy jest jakiś dobry system webowy, który polecacie?

Temat: Code Review

A co zaproponował Wasz zespół? :-)

Temat: Code Review

U mnie w pracy po każdym commicie zespół dostaje maila z svn diff (akurat svn, ale pewnie dla innych SCM też się da). Gdy ktoś ma jakieś wątpliwości to prosi kogoś by spojrzał do tego maila i powiedział co o tym myśli. To jedna z praktyk.

Mamy też swój szablonik jak powinien wyglądać raport z inspekcji kodu. Obecnie mało go stosujemy, kiedyś był popularniejszy. Plik tekstowy powstały według szablonu był umieszczany w repo, obok katalogu ze źródłami aplikacji. W ten sposób raczej nie da się go zgubić.

Ważne według mnie jest to by zespół zrozumiał ideę inspekcji kodu. By nie było tam osobistych wycieczek (tego lubię bardziej to się nie czepiam, a temu teraz dojeb...). Do każdej wątpliwości w kodzie powinny być jakieś argumenty. "Zmień to bo mi się nie podoba i już" to żaden powód by zmieniać.

Powinna być też jakaś rotacyjność w przeglądaniu kodu. Tak by nie powstały trwałe pary sprawdzany - sprawdzający.

Gdy dwóch programistów nie może się porozumieć, każdy z nich ma swoje racje powinien być jeden guru, który decyduje. Taki lead developer.

Powodzenia!

Temat: Code Review

Sebastian, bardzo dobrze piszesz. Tylko do samego poczatku mam troche watpliwosci. Przez te maile nie macie czasem zbyt duzego naplywu informacji ?
Osobiscie przegladam zmiany w kodzie przy kazdym "update" z repozytorium.

Temat: Code Review

Sebastian Szulc:
A co zaproponował Wasz zespół? :-)

Zespół nie znalazł nic wygodniejszego w użyciu od git-log / git-show / git-diff, tudzież przeglądarek commitów dostarczanych z softem do zarządzania projektem (Redmine, Assembla itp.) :(

Temat: Code Review

Zwykle robimy review na dwóch warstwach: design i code.

Design review przydaje się bardziej, gdy wiedza i doświadczenie deweloperów różni się znaczniej. Robią go zarówno młodsi (żeby się nauczyć), jak i starsi (żeby nauczyć).

Code review robiony jest po commit poprzez dwie osoby: dewelopera i rotacyjnie wybranego kolegę. Robią to razem na tym samym komputerze; takie pair-post-programming.

Pomocą służą diffy i wytyczne coding standards.

Jest to też metoda budowania współpracy, zaufania i otwartości w zespole.

Temat: Code Review

Pawel K.:
Przez te maile nie macie czasem zbyt duzego naplywu informacji ?

Nie. Nie mamy tego problemu

Temat: Code Review

Marek Kirejczyk:
Hej,

Wprowadzamy w naszej firmie code review.
Od czego zacząć?
Macie jakieś ciekawe doświadczenia, rady?
Czy jest jakiś dobry system webowy, który polecacie?

To trochę szeroki temat ;) Poza tym pytasz jakoś w kontekście Scrum?

Ile macie ludzi, jaki zapas effortu, jaki jest cel wprowadzania procesu (wprowadzacie "bo tak" czy macie "triggery", np. wykryta słaba jakość, duże zróżnicowanie doświadczenia, itp) oraz co chcielibyście tym uzyskać, jakiego używacie systemu wersjonowania, itd, itp?

Temat: Code Review

Także zróżnicowanie doświadczenia, ale przede wszystkim w dobrym zespole zawsze znajdzie się ktoś, kto wpadnie na pomysł "jak zrobić to lepiej" :)

Kontrola wersji -- git. Czasem jakiś klient ma projekt w subversion, ale wtedy szybko robimy konwersję repo do gita i na tym pracujemy.

Małe zespoły, 2-4 developerów na projekt.

Temat: Code Review

Jeśli narzędzie działa wygodnie to używajcie dalej (nie znam go). Spotykałem się z jakimiś webowymi rozwiązaniami (codestriker bodajże) ale jakoś się nie przyjmowały, lepiej wypadało review robione "normalnym" diffem na komputerze reviewera w danym systemie wersjonowania i raport z review w template w excelu.

Przy takiej ilości ludzi da się zrobić chyba tylko "jeden na jednego" (biorąc pod uwagę wypowiedzi powyżej, czyli np. jakieś systemy rotacji itp). Przy większych zespołach i silniejszej hierarchizacji można robić "ważniejszych" (lead developer, software architect, itp) odpowiedzialnych za review, którzy wtedy stają się quality gate'ami (tj. "przepuszczają", lub nie, kod).

Warto też pomyśleć nad grubymi review ważnych już działających modułów (choć nie znam specyfiki Ruby, może to jakiś bezpieczny język gdzie nie ma to sensu). Wtedy można robić też teamowo, tj. jest autor kodu, moderator i np. dwóch zawodników dodatkowych i wspólne review rzutnikiem.

Idąc dalej możecie oczywiście wprowadzić mierniki, dwa podstawowe to ilość reviewowanego kodu i czas nad tym spędzony, warto też dołożyć ilość findings. Może się to przydać w przyszłości do analiz procesu. Mierniki te można zwyczajnie dodać do szablonu zapisu z review. Po miesiącu czy dwóch można dzięki temu sprawdzić jak pięknie i do niebotycznych rozmiarów wzrasta liczba linii na godzinę ;) (co oczywiście nie jest dobrym znakiem).

Potem tylko ustalenie że np przed każdym check-inem (czy jak to nazwiemy) jest obowiązkowe review, że powstaje zapis, jakie warunki muszą być spełnione by check-in mógł nastąpić (np. wszystkie findings poprawione lub skomentowane z akceptacją reviewera) i ustalenie miejsca składowania zapisów i ich łączenia z wersją.

Tyle od strony procesu bo całkowicie osobna kwestia to to CO jest reviewowane.

Można reviewować spełnianie jakiś coding rules, np. nazewnictwo zmiennych, wcięcia, długość czy forma nazw, ogólna estetyka kodu itd. To jest podstawa. Następny poziom to review logiki, tu schodzi się już do powiedzmy 200 linii/godzinę ale to właśnie chyba chcecie osiągnąć (apropos "jak zrobić to lepiej"). Jeśli jest review logiki a wierzycie że design i module testy są ważne ( ;) ) to można robić też review spójności kodu z designem i module testami.

I do takiego czegoś przydają się checklisty. Możecie na początku siąść i zastanowić się nad tym co da największe korzyści albo co powodowało najwięcej problemów w przeszłości i stworzyć rzeczy obowiązkowe do sprawdzenia spisując je w formie checklisty. Przykładowo dla C: "każdy wskaźnik jest inicjowany przed pierwszym użyciem" albo "każde użycie memcopy poprzedzone jest sprawdzaniem zakresu" itp.

Chyba nic ciekawego na starcie się nie podpowie więcej, teraz już musicie obserwować jak to działa i czy coś daje. Ciężko też bez konkretnego celu czy powodu bo wtedy proces niby jest wdrożony ale nie bardzo wiadomo czy działa i tak jakoś "jest bo jest", każdy wie że to dobrze ale nie wiadomo konkretnie jakie są korzyści.Rafał D. edytował(a) ten post dnia 22.07.09 o godzinie 00:12

Temat: Code Review

Wow. Dzięki. :)