Jak skłonić programistów do szybkiego dokonywania recenzji kodu

12

Firma, dla której pracuję, wymaga sprawdzenia całego kodu przez innych programistów przed jego zatwierdzeniem. Członkowie mojego zespołu są często sfrustrowani, ponieważ inni programiści są zbyt zajęci kodowaniem, aby dokonać przeglądu, szczególnie jeśli jest on bardzo długi. Jak zachęcasz innych programistów do dokonywania terminowych recenzji kodu?

(Używamy git-svn, abyśmy mogli kontynuować kodowanie w oczekiwaniu na recenzję. Jednak wciąż frustruje mnie, gdy muszę długo czekać, zanim będę mógł zatwierdzić swój kod).

titaniumdecoy
źródło

Odpowiedzi:

12

Zobacz, jak robi to Facebook z własną aplikacją o nazwie phabricator: http://phabricator.org/

Zasadniczo zatwierdzają dla poszczególnych numerów, a dla każdego numeru wyświetlany jest kod, który ktoś ma sprawdzić. Kod nie trafia do ich głównego repozytorium, dopóki recenzent nie stwierdzi, że jest to w porządku.

Myślę, że to sprawia, że ​​jest to bardziej zabawne.

Być może kod powinien być przypisany dwóm osobom: jednej, która to robi, i drugiej, która go recenzuje.

Chociaż być może twoi koledzy z drużyny nie wierzą w tę recenzję.

Osobiście, z braku recenzentów, użyłem testów jednostkowych dla funkcji niższego poziomu i „testu woźnego” dla całej reszty: test woźnego jest tak nazywany, ponieważ nawet woźny powinien być w stanie zrozumieć Twój kod.

Zwykle usuwałem niektóre drobne części, takie jak nawiasy blokowe / funkcyjne, notki widoczności, czasem nawet typy, i pokazywałem to menedżerom, ekspertom domeny, partnerom, którzy poprosili o kod: „czy tego chcesz?”

Pomaga też osobiście udać się tam i nie wyjechać, dopóki nie dokona się przeglądu.

Lub, jeśli nie masz nic przeciwko zespołowi lub nie są w porządku, wiesz, „jeśli możesz zmienić firmę, zmienić firmę” ...

Aadaam
źródło
9

Opieram to na kilku założeniach:

  1. Wydaje się, że wszyscy chcą pisać kod (jeśli nie, masz ludzi, którzy muszą iść).
  2. Wszyscy chcą, aby ich kod został sprawdzony.

Zezwalaj tylko tym, którzy wypełniają recenzje, na sprawdzenie kodu.

Być może pewien blok czasu można poświęcić na przeglądanie kodu w nadziei uniknięcia problemu przerwania.

Celem powinno być sprawdzenie kodu jakości. Nie chcesz karać / wymuszać recenzji do tego stopnia, że ​​wszyscy po prostu przyznają jej „pieczątkę”.

Jeśli masz różne poziomy (jr., Sr. Itp.), Awans i utrzymanie tytułu powinny być uzależnione od wykonania pracy.

JeffO
źródło
1

W przypadku kilku poprzednich pracodawców rotowaliśmy, którzy codziennie korzystali z Code Review. Zwykle 3 osoby dzieliły dzień CR i każdy CR musiał być podpisany przez dwóch recenzentów. To sprawiło, że kiedy był twój dzień, wiedziałeś, że CR będzie od ciebie oczekiwany, niezależnie od innych projektów. Co około pięć dni nadeszła Twoja kolej i możesz odpowiednio dostosować swoje zadania programistyczne.

Obecnie mamy kierownika zespołu, który pobierze pobieżnie CR na kodzie swojego zespołu. W zależności od tego, który obszar aplikacji został zaktualizowany, po zakończeniu CR można go podnieść do Globalnego Zespołu Przeglądu, w którym sprawdzamy, czy istnieją rzeczy, które mają globalny wpływ na aplikacje, zamiast rzeczy, które są związane z jedną funkcją. Kiedy trzeba dokonać dużej recenzji, zwykle dzielimy ją na kilka osób, aby nikt nie musiał radzić sobie ze zmianami w absurdalnej liczbie plików.

To powiedziawszy, zawsze sprawdzamy tylko kod, który został przypisany do aktualnej gałęzi / wariantu Dev. Kod musi przejść przegląd kodu, przegląd globalny, przegląd DB i przegląd interfejsu użytkownika (każdy w razie potrzeby), zanim będzie można go awansować do następnego środowiska (np. Alpha).

Wreszcie, zgadzamy się na umowę SLA dotyczącą tego, jak szybko zmieniane są Recenzje. Prawie nic nie stoi w kolejce przez ponad 48 godzin, a większość recenzji jest wykonywana w mniej niż 24 godziny.

Adrian J. Moreno
źródło
1

Firma, dla której pracuję, wymaga sprawdzenia całego kodu przez innych programistów przed jego zatwierdzeniem. Członkowie mojego zespołu są często sfrustrowani ...

O ile nie tworzysz oprogramowania o znaczeniu krytycznym lub poprawek krytycznych do kodu kandydata do wydania produkcyjnego, nie ma istotnych powodów, by ściśle trzymać się określonego rodzaju recenzji kodu.

  • Idea stojąca za wymaganiami Twojej firmy brzmi rozsądnie na pozór (100% sprawdzony kod), ale środki, które zdecydowali się zastosować, przynoszą efekt przeciwny do zamierzonego - ponieważ, jak zauważyłeś, prowadzą do frustracji programistów.

Wchodząc w twoje buty, chciałbym najpierw wskazać kierownictwu, że recenzje kodu po zatwierdzeniu są uważane za równie godne szacunku, jak te przed zatwierdzeniem . Terminy te można przeszukiwać w Internecie - w razie potrzeby znajdź odniesienia do kopii zapasowej tego. Aby uzyskać 100% sprawdzonego kodu, nie trzeba koniecznie dokonywać przeglądu przed zatwierdzeniem .

W oparciu o powyższe zasugerowałem następnie, aby porzucili podejście mikrozarządzania i pozwolili programistom wypróbować sposób, który będzie im wygodniejszy. Recenzje przed lub po zatwierdzeniu najlepiej pozostawić programistom do wyboru. Jeśli firma wie lepiej niż programiści, dlaczego nie sami piszą kodu?

komar
źródło
1
„Jeśli firma wie lepiej niż programiści, dlaczego nie sami piszą kodu?”: Bardzo dobry komentarz! Mam jednak nadzieję, że kierownicy ds. Rozwoju sami są dawnymi programistami.
Giorgio
3
Z mojego doświadczenia wynika, że ​​po zatwierdzeniu okropnie szkodzi jakości twojego kodu. Programiści są leniwi i poczują, że są skończeni, jeśli można to popełnić: „tak, cóż, to nie jest idealne, ale kogo to obchodzi, co jest idealne w życiu? To działa, prawda? „ jedyną dobrą odpowiedzią jest NIE, a być może menedżerowie mają bardziej realistyczny obraz programistów niż o sobie, dlatego wymagają przeglądu kodu przed zatwierdzeniem (a przynajmniej scalenia).
Aadaam
@Aadaam twoje doświadczenie zdecydowanie różni się od mojego - nie tylko w odniesieniu do post-commits, ale także (a zwłaszcza) wrt części „Programiści są leniwi ...” Jeśli chodzi o menedżerów mających bardziej realistyczny wizerunek, zgadzam się, że zwykle tak było moje zespoły; nie doprowadziło to tylko menedżerów, z którymi pracowałem, do bezsensownych pomysłów na temat tego, jaką recenzję wymusić
gnat
Pracowałem w outsourcingu. W przypadku outsourcingu większość programistów nie jest zaangażowana, ponieważ programowanie jest fajne, ale dlatego, że programowanie ma najlepszy stosunek pracy do wynagrodzenia, a stawki są znacznie wyższe niż cokolwiek innego ... nienawidzą tego jak każda inna praca ... i spróbuj zrobić wszystko, aby dalej „zoptymalizować” ten stosunek, jeśli wiesz, co mam na myśli…
Aadaam
1

Masz wiele problemów do rozwiązania - musisz zdobyć serca i umysły oraz upewnić się, że masz czas na recenzje kodu.

Druga część jest prawdopodobnie najłatwiejsza - zgadzasz się (łącznie i to musi obejmować zarządzanie), że pierwszą rzeczą, którą programista robi każdego ranka, są recenzje kodu - jest to proste, zrozumiałe, skuteczne i daje ci fajny kij do pokonania ludzi jeśli nie są zgodne. Co więcej, niczego nie przerywasz, nie prosisz ich, aby przestali pracować nad swoim kodem, nie prosisz ludzi o wyciśnięcie czegoś z listy rzeczy do zrobienia ...

Pierwsza część to prawdziwy problem - uczestnicy procesu sprawdzania muszą widzieć, że ma wartość, w przeciwnym razie nigdy nie zrobią przeglądu kodu (który jest postrzegany jako nie mający wartości), gdy mogą pisać kod lub naprawiać błędy (które jest z pewnością ważniejsze ...?).

Jeśli umiesz połączyć te dwa elementy - po pierwsze upewniając się, że wszyscy wierzą (lub rozumieją), że w recenzjach kodu ma wartość - w najprostszym przypadku powinno to oznaczać mniej błędów, co oznacza więcej nowego kodu, co zwykle sprawia więcej zabawy, a następnie po drugie porządkowanie rzeczy, aby w harmonogramie było wolne miejsce na przegląd kodu, mam nadzieję, że wydarzy się coś dobrego ... stanie się częścią kultury.

Kiedyś będzie to część kultury, może nie być już konieczne mówienie „pierwsza rzecz każdego dnia” - ale powiedziawszy, że uważam, że pasuje do wzorca, w którym prawdopodobnie chce pracować deweloper.

Murph
źródło
Nie możesz naprawdę zgodzić się, że zasada „pierwsza rzecz każdego dnia” jest najważniejsza. Jeśli ktoś znajdzie błąd, który kosztuje firmę X dolarów na godzinę (lub zwiększa ryzyko pominięcia ważnego terminu o X punktów procentowych na godzinę), a zdarza się to na pięć minut przed wejściem, przegląd kodu nie jest twoim najwyższy priorytet. Zasadniczo problemem jest zderzenie między chęcią ustanowienia prostych zasad a faktem, że priorytety nie zawsze są proste. Istnieje ryzyko, że wszyscy całkowicie zgodzą się na regułę i w ciągu 24 godzin znajdą jakiś powód, dla którego dzisiejszy dzień jest wyjątkiem od tej reguły.
Steve Jessop,
Rozwiązanie jest trudne, ale IME musi znaleźć wystarczającą „przestrzeń”, aby wprowadzić nową praktykę pracy, która jest czasochłonna, ale opłacalna. Wymaga to dalekowzroczności, aby zidentyfikować opóźnienie, chęć pozostawienia terminów jako ceny wprowadzenia wartościowej zmiany lub obu. TANSTAAFL. Jak mówisz, gdy wszyscy ustalą, że będą mogli zrobić wyjątek. Mamy nadzieję, że robią to w oparciu o właściwe uznanie wartości ogólnego wzorca.
Steve Jessop,
Mówię „pozwól terminom minąć”, powinienem był powiedzieć „przesunąć terminy”. „poślizg” oznacza przeniesienie ich po popełnieniu, tzn. niepowodzeniu, ale nie musi tak być. Zamiast tego możesz zaplanować okres nieco obniżonej produktywności z powodu nieelastycznej nowej reguły (i nieuniknionej nieefektywności spowodowanej przez osoby próbujące śledzić każdy nowy proces - jeśli najpierw robisz przegląd kodu, przegapisz poranny scrum spotkanie w dni przegląd kodu trwa zbyt długo lub jakiekolwiek inne drobiazgi, które Twoja organizacja może wrzucić do miksu). Jeśli to dobra zasada, wkrótce znajdziesz się powyżej miejsca, w którym zacząłeś.
Steve Jessop,
@ SteveJessop oczywiście możesz się z tym zgodzić. I oczywiście będą wyjątki (zdarza mi się, że obserwacja scrum jest głupia - zwłaszcza, że ​​odpowiedź jest oczywista (-:). Myślę, że kluczem jest to, że nie ma jednego „uniwersalnego rozwiązania”, po prostu zaproponował coś, co jest proste i łatwe do zrozumienia i jest stosunkowo trudne do podważenia własnego harmonogramu (ponownie w zależności od środowiska)
Murph
1

W większości firm, dla których pracowałem, masz 3 dni na dokończenie recenzji. Niedopuszczalne jest nie robienie recenzji. To część twojej pracy. Jeśli nie zrobisz przyzwoitych recenzji na czas, wpłynie to na twoją ocenę wydajności. I tak, recenzje zawsze wydają się zdarzać w najbardziej nieodpowiednich momentach. Szkoda, naucz się uwzględniać czas oceny w swoich prognozach. W każdym razie, jeśli kierownictwo naprawdę wierzy, że recenzje są ważne (tj. Nakazują przeglądanie całego kodu), wówczas zastosowaliby podobną politykę. Dodatkowo, jeśli ludzie nie ukończą przeglądu w wyznaczonym czasie, oznacza to akceptację materiału.

Maczać
źródło
0

Rozważ skorzystanie z narzędzia takiego jak komisja recenzująca . Jest to bardzo pomocne, szczególnie w przypadku długich recenzji.

Możesz przesłać swoje różnice i poczekać, aż recenzent zakończy swoją recenzję. Jeśli masz otwarte recenzje, które uniemożliwiają ci kontynuowanie pracy, możesz to zgłosić podczas codziennych spotkań (Twój zespół chce, aby nowe funkcje zostały sprawdzone, aby można je było jak najszybciej przetestować, prawda?).

Giorgio
źródło
0

Kilka punktów do dodania, których nie ma w innych odpowiedziach.

Kod do przejrzenia musi zostać zameldowany

  • tak, że przeglądasz stabilną wersję.
  • Może znajdować się w głównej gałęzi programistycznej, jeśli wydanie jest wystarczająco daleko
  • Może być na gałęzi, jeśli istnieje dobry powód, aby nie zanieczyszczać magistrali

Blokowanie zadań ma pierwszeństwo, dlatego recenzje kodu powinny mieć pierwszeństwo przed innymi pracami (ale starając się nie przerywać przepływu). Jako programista powinieneś poprosić innych o sprawdzenie Twojego kodu (ponieważ chcesz go ulepszyć). Mając tę ​​wiedzę, należy niezwłocznie wykonywać recenzje dla innych osób.

Trudniejsze pytania dotyczą tego, kiedy i jak dobrze wykonywać recenzje kodu.

Reguła, która zadziałała dla nas w tym momencie, jest taka, że ​​kod dzielony musi zostać sprawdzony, ponieważ ma szerszy wpływ, podczas gdy w kodzie dla pojedynczej aplikacji (szczególnie biorąc pod uwagę, że używamy programowania opartego na testach) jest mniej ważny.

Bruce Adams
źródło