Jak skutecznie monitorować przegląd kodu?

28

Podejrzewam, że w moim zespole ukrywa się duże recenzje kodu. Zbyt wiele recenzji kodu jest scalanych bez komentarza.

Wydaje mi się, że nie ma czegoś takiego jak przegląd kodu bez jednego komentarza.

Jak mogę jako lider zespołu odpowiednio monitorować, czy mój zespół przeprowadza prawidłowy proces przeglądu kodu i jak mogę pomóc im zmaksymalizować korzyści płynące z tego procesu?

Aktualizacja

Myślałem, że ludzie mogą chcieć wiedzieć o każdej aktualizacji. Wypróbowałem wiele podanych tutaj sugestii. większość była już w użyciu. niektórzy trochę pomogli. Problem jednak pozostał - niektórzy ludzie ciągle dostają zły kod, gdy nie patrzyłem.

Przekonałem się, że monitorowanie recenzji kodu nie jest tak pomocne, jak udostępnienie narzędziom mojego zespołu, aby lepiej zacząć od kodu.

Dodałem więc bibliotekę o nazwie „jscpd”, aby wykryć kopie past. Kompilacja nie powiodła się na pastach kopiujących. To natychmiast wyeliminowało jeden problem.

następnie spróbujemy zastosować kodeklimat.

Robię też ręczną recenzję starych recenzji kodu raz na pół dnia. Przekształcam todos na problemy / bilety - jak się dowiedziałem, ludzie je piszą, ale nigdy nie są obsługiwane. Robię też spotkania z całymi zespołami, aby sprawdzić kod, gdy jest to właściwe.

ogólnie wydaje się, że zmierzamy we właściwym kierunku.

facet mograbi
źródło
1
Jeśli używasz TFS, możesz go skonfigurować tak, aby zawierał Nazwę recenzenta kodu.
Krishnandu Sarkar
11
@gnat Nie zgadzam się. Istnieje różnica między osobą, która nie lubi recenzji kodu, a tym, o co pyta to pytanie. To pytanie można zaatakować z perspektywy identyfikowalności (łączenie zmian w kodzie źródłowym z recenzją lub wad / ulepszeń / historii z recenzjami tej implementacji itp.) Lub z perspektywy jakości procesu i kontroli. Oba mają implikacje, nawet jeśli ludzie na ogół nie mają problemu z przeglądaniem kodu.
Thomas Owens
3
Czy bierzesz udział w którejkolwiek z tych recenzji? Może czas wpaść na jednego? Wskaż kilka rzeczy sam i zapytaj każdego recenzenta indywidualnie, dlaczego przegapił je wszystkie?
Mawg,
2
Czy uważasz, że recenzja nie zauważyła oczywistych problemów? Czy ty dodałeś (Ważny) komentarze?
usr

Odpowiedzi:

70

Mam zamiar zaoferować inne podejście niż moi koledzy. Mają rację - zaangażuj się, jeśli chcesz zobaczyć, jak się sprawy mają. Jeśli chcesz mieć większą identyfikowalność, istnieją do tego narzędzia.

Ale z mojego doświadczenia podejrzewam, że dzieje się coś jeszcze.

Czy uważasz, że Twój zespół może uważać, że proces jest zepsuty / głupi / nieskuteczny w przypadku większości zmian? Pamiętaj, że proces dokumentuje to, co działa dobrze, a nie zasady, których należy przestrzegać . Jako lider zespołu, jesteś tam, aby pomóc im być najlepszym, a nie egzekwować zasady.

Więc w swoich retrospektywach (jeśli zwinny) lub jeden na jednego (jeśli jesteś menedżerem) lub na przypadkowych zaimprowizowanych spotkaniach na korytarzu (jeśli nie jesteś zwinnym liderem zespołu i inny menedżer robi to jeden na jednego), przywołaj to . Zapytaj, co ludzie myślą o procesie sprawdzania kodu. Jak to działa Jak to nie jest Powiedz, że myślisz, że może nie przynosi to tak dużej korzyści zespołowi, jak mogłoby. Upewnij się, że słuchasz .

Podczas tych spotkań możesz popierać recenzje kodu, ale lepiej posłuchać opinii. Najprawdopodobniej przekonasz się, że albo zespół uważa, że ​​„właściwy” proces wymaga dostosowania, lub że istnieje jakaś podstawowa przyczyna (presja czasu, brak recenzentów, Bob po prostu popełnia swój kod, więc dlaczego nie możemy) rozwiązać .

Wymuszenie narzędzia na wierzchu zepsutego procesu nie poprawi go.

Telastyn
źródło
5
+1 za właściwe podejście do tego (i wielu innych!) Problemu
Olivier Dulac
7
+1 za ostatnie zdanie. Jest to coś, czego prawie nikt nie rozumie, ale jest niezwykle ważne.
JohnEye,
1
Niezła odpowiedź. Próbowałem tego ... Mój zespół mówi: „Firma robi rzeczy w niewłaściwy sposób. Potrzebujemy więcej qa .. i pozwalamy programistom się rozwijać”, podczas gdy firma mówi „Chcemy, aby programiści przesyłali kod dobrej jakości. Naszym celem jest rozproszenie zespołu qa ponieważ gdy kod jest dobrej jakości, qa nie jest już potrzebna ...
facet mograbi
43

Nie lubię publikować odpowiedzi w jednym wierszu, ale ta wydaje się odpowiednia:

Weź udział w procesie.

Blrfl
źródło
15
Nie lubię też odpowiedzi na jedną linię. Na szczęście wybrałeś dwie linie - i moja odpowiedź. +1
Mawg
1
Jestem. ale kiedy nie jestem ... coś się dzieje. właśnie to wzbudziło moje podejrzenia. Zacząłem ponownie recenzować recenzje innych i znalazłem nieprzyjemne rzeczy.
facet mograbi
6

Uzyskaj narzędzie, takie jak ReviewBoard lub wtyczka Redview do kodowania . Następnie każda recenzja jest tworzona jako zadanie, które musi zostać zamknięte lub skomentowane przez kogoś (podobnie jak zgłoszenie błędu). Następnie masz możliwość śledzenia, kto utworzył bilet recenzji, a kto go zamknął. Możesz powiązać bilety przeglądu z rejestracją kodu źródłowego, tj. Utworzyć bilet z wersji.

gbjbaanb
źródło
2

Kilka rzeczy (szczerze mówiąc, większość z nich obejmuje odpowiedzi, ale chciałem umieścić je w jednym miejscu)

  • Możesz wdrożyć proces i reguły, aby mieć pewność, że nastąpi przegląd kodu, ale jest to prawie niemożliwe, aby przeglądanie kodu było w rzeczywistości czymś więcej niż tylko zaznaczaniem pola. Ostatecznie zespół musi zobaczyć korzyści płynące z tego procesu, jeśli ma do niego podejść w sposób użyteczny

  • Dawaj dobry przykład. Weź udział w recenzjach. Jako programista czuję się źle, jeśli mój menedżer (obecnie nie-programista) dostrzega rzeczy, których nie rozumiem. Podkreśl problemy, które powinny zostać uwzględnione w przeglądzie (bez obwiniania). Jeśli wystąpi problem z produkcją, jeśli pojawią się problemy podczas kontroli jakości (jeśli masz osobny proces kontroli jakości), zaznacz, gdzie mogły zostać złapane podczas przeglądu kodu. Omów z zespołem, w jaki sposób możemy zapewnić, że takie problemy zostaną w przyszłości wykryte

  • Omów z zespołem, co chcą zrobić. Jeśli nie widzą sensu (co może się zdarzyć na początku), użyj problemów z produkcją i koniecznych poprawek jako dowodu korzyści

  • Korzystaj z oprogramowania do automatycznego sprawdzania kodu, takiego jak Sonarqube, aby recenzje kodu koncentrowały się na takich kwestiach, jak niezrozumiały kod, błędy logiczne, brak dokumentacji itp., Których nie można wykryć automatycznie.

matowy freake
źródło
2

Możesz udokumentować, co zespół chce w recenzjach kodu, które omówiłeś i zgodziłeś się z programistami. Oto niektóre rzeczy, które możesz rozważyć w ramach recenzji kodu:

  • Sprawdź, czy kod robi to, co powinien, tzn. Spełnia wymagania

  • Styl kodu zapewniający, że programiści kodują spójny styl

  • Optymalizacja np. Liczba wywołań funkcji

  • Architektura i możliwość ponownego użycia

  • Obsługa wyjątków i rejestrowanie

  • Dług techniczny: kod jest w lepszym stanie niż w momencie, gdy programista zaczął nad nim pracować

  • Sprawdź i zbuduj kod (uważam to za przydatne, ale inni deweloperzy w moim zespole wolą zostawić to testerom)

  • Za pomocą zautomatyzowanego narzędzia (użyłem SonarQube ). Uważam za użyteczne zintegrowanie tego z procesem kompilacji w celu wymuszenia ulepszeń kodu, np. Zwiększenia zasięgu testu

Niektóre z powyższych kroków można wykonać za pomocą automatycznego narzędzia, ale podczas próby ulepszenia sposobu dokonywania recenzji kodu lub wykonywanych czynności prawdopodobnie warto używać zarówno narzędzia, jak i przeglądu gałek ocznych. Jednak najważniejsze kroki mające na celu zapobieganie zadłużeniu technicznemu (architektura i możliwość ponownego użycia) nie mogą być całkowicie zautomatyzowane.

Jeśli Twój zespół nie jest konsekwentny w stosowaniu tego, możesz tylko zezwolić programistom, którzy prawidłowo przeprowadzają recenzje kodu, na posiadanie praw do scalania. Na przykład możesz po prostu zacząć od głównego programisty w zespole. Kompromis z tym podejściem polega na tym, że ci programiści mogą stać się wąskim gardłem w procesie rozwoju, więc ty i zespół musicie zdecydować, czy tego chcecie. Osobiście zaakceptowałbym ten kompromis i dzięki przeglądom kodu zwiększyłem dyscyplinę w pozostałym zespole, a kiedy będziesz gotowy, możesz zwiększyć liczbę programistów z prawami do scalania.

Na koniec warto przejrzeć recenzje. Raz w tygodniu spotykaj się z programistami i konstruktywnie omawiaj recenzje oraz sposoby ich ulepszenia.

br3w5
źródło
czy to jest reklama SonarQube? Próbowałem - nie poleciłbym tego, zbyt bolesne, aby zacząć, a „open source” kosztuje wszystkie przydatne bity.
gbjbaanb
Działa dobrze w moim obecnym zespole i nie było zbyt trudne do skonfigurowania i pomaga - to nie jest reklama, ale to jedyne tego rodzaju narzędzie, z którego mam doświadczenie. Czy powiedziałbyś to samo o widoku kodu Redmine i ReviewBoard?
br3w5,
Używamy SonarQube w naszych zespołach, obsługując ponad 70 projektów, od 10 000 do 3 mln LOC. Chociaż niektóre zespoły po prostu ignorują swoje raporty, większość używa go do bezpośredniego refaktoryzacji procesów. Działa dobrze, chociaż osobiście wolę proste, niezintegrowane narzędzia, takie jak Findbugs.
Dibbeke,
I oto pomyślałem, że przegląd kodu
polega na
1
Dzięki, to właśnie robię w międzyczasie. Zaktualizuję w ciągu kilku tygodni, jak to wpłynęło.
facet mograbi
0

Opowiem ci, jak mój zespół szybko zintegrował przegląd kodu z przepływem pracy.

Najpierw pozwól, że zadam ci pytanie. Czy używasz systemu kontroli wersji (np. Mercurial, Git)?

Jeśli Twoja odpowiedź brzmi „tak”, kontynuuj.

  1. zabronić każdemu pchania czegokolwiek (nawet drobnych poprawek) bezpośrednio do gałęzi głównej (pnia) *
  2. rozwijać nowe funkcje (lub poprawki) w osobnych gałęziach
  3. gdy programiści uważają, że gałąź jest gotowa do zintegrowania z master, utworzą „żądanie ściągnięcia”
  4. zabraniają wszystkim scalania własnego żądania ściągnięcia *
  5. poproś innego programistę o ocenę żądania ściągnięcia i sprawdzenie nowego kodu
  6. jeśli kod przejdzie sprawdzenie, dobrze, żądanie ściągnięcia można scalić, w przeciwnym razie można zastosować poprawki
  7. powtarzaj krok 6, aż kod wystarczająco dojrzeje (można to zrobić bez rozpoczynania od nowa) **
  8. gotowe, cały Twój nowy kod zostanie sprawdzony (przynajmniej w skrócie) przez osobę o nazwisku

Teraz masz precyzyjny punkt w przepływie pracy, w którym dokonuje się przeglądu kodu.

Działaj tam.

* może być egzekwowany automatycznie za pomocą haków po stronie serwera

** ta procedura jest w pełni obsługiwana przez GitHub (między innymi) i jest dość łatwa w użyciu, sprawdź ją

Agostino
źródło
2
Nawet przy takim procesie (który, jak przypuszczam, dzieje się na podstawie opisu w pytaniu), czasami programiści myślą „ah, ufam wystarczająco mojemu koledze i mam zbyt wiele do zrobienia, więc po prostu scalę go bez czytania szczegóły, a nawet komentowanie ”. (Mamy podobny proces w naszym zespole,
wymagamy
1
@ PaŭloEbermann Rozumiem. Obawiam się, że jest to nieunikniony wynik okoliczności, jeśli nie masz wystarczająco dużo czasu na zrobienie wszystkiego, czego potrzebujesz, jakość ucierpi w ten czy inny sposób. Sill, jeśli nie działa „czasami”, oznacza to, że działa „przez większość czasu”, nie?
Agostino,
1
Tak, pomogło to trochę, pozwalając na scalenie tylko ograniczonej grupie osób, które miały za zadanie sprawdzić, czy właściwa recenzja została wykonana poprawnie.
Paŭlo Ebermann
Miałem podobny zakaz i nie trzeba dodawać: rozwój prawie się zatrzymał. Zasada ta trwała całe 2 tygodnie, po których menedżerowie musieli dostosować swoje plany.
BЈовић
@ BЈовић był zespół robi opinie kodu regularnie przed ? Ta technika jest stosowana przez wielu, szczególnie w ekosystemie Open Source. To, że nie działało dla twojego zespołu, nie oznacza, że ​​nie może działać dla innych.
Agostino,
-2

Myślę, że powinieneś utworzyć szablon i poprosić członków zespołu o aktualizację go za każdym razem, gdy przeprowadzają przegląd kodu. Ale nawet wtedy powinieneś początkowo uczestniczyć w procesie przeglądu.

użytkownik85
źródło