Jaki jest najskuteczniejszy sposób przeprowadzania recenzji kodu? [Zamknięte]

71

Nigdy nie znalazłem idealnego sposobu na wykonanie recenzji kodu, a jednak często wymagają tego moi klienci. Wydaje się, że każdy klient robi to w inny sposób i nigdy nie czułem satysfakcji w żadnym z nich.

Jaki jest najbardziej efektywny sposób przeprowadzania przeglądów kodu?

Na przykład:

  • Czy jedną osobę uważa się za strażnika jakości i przegląda kod, czy też zespół jest właścicielem normy?
  • Czy przeglądasz kod jako ćwiczenie zespołowe przy użyciu projektora?
  • Czy odbywa się to osobiście, pocztą elektroniczną lub za pomocą narzędzia?
  • Czy unikasz recenzji i używasz takich rzeczy, jak programowanie parami i zbiorowe posiadanie kodu, aby zapewnić jakość kodu?
Paddyslacker
źródło
Smart Bear Software ma darmową książkę o nazwie Best Kept Secrets of Peer Code Review . Bezpłatnie z bezpłatną wysyłką. I chociaż robią to po okazjonalnym e-mailu sprzedażowym. Nie byli nachalni. A tak przy okazji ... książka jest całkiem dobra.
John MacIntyre

Odpowiedzi:

32

W mojej pracy mamy bardzo prostą zasadę: zmiany muszą zostać sprawdzone przez co najmniej jednego innego programistę przed scaleniem lub zatwierdzeniem do pnia . W naszym przypadku oznacza to, że druga osoba fizycznie siedzi przy komputerze i przegląda listę zmian. To nie jest idealny system, ale mimo to wyraźnie poprawił jakość kodu.

Jeśli wiesz, że Twój kod zostanie poddany przeglądowi, zmusi cię to do przejrzenia go w pierwszej kolejności. Widać wtedy wiele problemów. W naszym systemie musisz wyjaśnić recenzentowi, co zrobiłeś, co ponownie powoduje, że zauważysz problemy, które wcześniej przeoczyłeś. Ponadto, jeśli coś w kodzie nie jest od razu jasne dla recenzenta, jest to dobra wskazówka, że ​​wymagana jest lepsza nazwa, komentarz lub refaktoryzacja. I oczywiście recenzent może również mieć problemy. Ponadto, oprócz spojrzenia na zmianę, recenzent może również zauważyć problemy w pobliskim kodzie.

Istnieją dwie główne wady tego systemu. Gdy zmiana jest trywialna, nie ma sensu przeglądać jej. Musimy jednak bezwzględnie przestrzegać zasad, aby uniknąć śliskiego nachylenia deklarowania zmian jako „trywialnych”, gdy nie są. Z drugiej strony nie jest to bardzo dobry sposób na przegląd istotnych zmian w systemie lub dodanie dużych nowych komponentów.

Wcześniej próbowaliśmy bardziej formalnych recenzji, gdy jeden programista wysłał kod e-mailem do sprawdzenia dla reszty zespołu, a następnie cały zespół zebrał się i przedyskutował. Zajęło to dużo czasu wszystkim, w wyniku czego recenzje te były bardzo nieliczne i tylko niewielki procent bazy kodu został sprawdzony. „Jedna osoba sprawdza zmiany przed zatwierdzeniem” działała dla nas znacznie lepiej.

Dima
źródło
Bardzo przydatne, dzięki! Przygotowuję sesje własnego zespołu i myślę, że to może być dobre podejście.
Neonigma
13

Lubię recenzje kodu, choć mogą być uciążliwe. Powodem, dla którego je lubię, jest to, że zwracają większą uwagę na kod i inną perspektywę. Uważam, że nawet przy programowaniu parami kod powinien zostać sprawdzony. Dwie osoby pracujące nad tym samym kodem mogą łatwo popełnić ten sam błąd, którego nie może zabraknąć w innym zestawie oczu.

Jeśli zrobiono to w grupie z projektorem, naprawdę powinno się to sprawdzić indywidualnie przed spotkaniem. W przeciwnym razie jest to po prostu irytująca strata czasu.

Zrobiłem recenzje kodu tylko przez e-mail i w grupie. Ogólnie rzecz biorąc, nie sądzę, że należy to robić osobiście. Czujesz trochę większą presję, aby pospiesznie przeszukiwać kod, gdy ktoś patrzy ci przez ramię. Wierzę, że narzędzie zaprojektowane do przeglądania kodu byłoby dobrym atutem, ponieważ może pomóc w niektórych przyziemnych aspektach i powinno ułatwić oznaczanie problematycznych fragmentów kodu, niż za pośrednictwem poczty elektronicznej.

Problem z tym, że jedna osoba wykonuje wszystkie recenzje kodu, polega na tym, że może to stanowić wąskie gardło. Przy dobrze udokumentowanych i zaprojektowanych standardach kodowania nie powinno to być konieczne. W zależności od środowiska / harmonogramu wydania dobrym pomysłem może być posiadanie kogoś jako osoby sprawdzającej kod w trybie gotowości.

Wierzę, że własność kodu jest dobrym pomysłem, ponieważ ta osoba może uznać za swój priorytet zrozumienie tego kodu i potencjalnie odgrywać rolę strażnika.

George Marian
źródło
+1 za „Jeśli wykonano je w grupie z projektorem, naprawdę powinno się je przejrzeć indywidualnie przed spotkaniem. W przeciwnym razie jest to po prostu irytująca strata czasu.”
AShelly
1
„Wykonanie w grupie z projektorem jest irytującą stratą czasu”. Tam to naprawiłem.
gbjbaanb
Kiedy robisz to osobiście, jest to inny proces, to nie ty przeglądasz kod drugiego, gdy on patrzy przez ramię. To on wyjaśnia wiersz po wierszu, co zmienił, co robi jego zmiany i dlaczego, kiedy go słuchasz. Wywiera presję na to, kto stworzył kod, aby go wyjaśnić, a nie na ciebie, aby go zrozumieć i przejrzeć.
Didier A.,
6

W SmartBear tworzymy nie tylko narzędzie do sprawdzania kodu , ale także używamy go na co dzień. Jest to niezbędna część naszego procesu rozwoju. Sprawdzamy każdą zmianę, która zostanie sprawdzona.

Myślę, że to zły pomysł, aby mieć jednego recenzenta kodu strażnika z wielu powodów. Ta osoba staje się wąskim gardłem i musi robić zbyt wiele recenzji kodu (tylko po to, aby projekt był w ruchu), aby być naprawdę skutecznym (60-90 minut na raz jest granicą skuteczności). Recenzje kodu to świetny sposób na dzielenie się pomysłami i wiedzą. Bez względu na to, jak wielką gwiazdą jest twój strażnik, mogą uczyć się od innych członków zespołu. Ponadto, każdy, kto dokonuje przeglądu kodu, tworzy również środowisko „zbiorowego posiadania kodu” - w którym ludzie czują, że są właścicielami jakości kodu (to nie tylko kontrola jakości lub strażnik).

Mamy bezpłatny oficjalny dokument na temat „ Najlepszych praktyk w zakresie recenzowania kodu równorzędnego ”, który zawiera 11 wskazówek, jak skutecznie przeprowadzać recenzje kodu. Wiele z nich to te same treści z książki, o których John wspomniał w bardziej destylowanej formie.

Brandon DuRette
źródło
1
Link w dół ...........
Pacerier
Przepraszam za zgniliznę linku. Zmieniłem bieżący adres URL, ale nie zapobiegnie to powtórzeniu się.
Brandon DuRette
3

Bez wymówek. Ćwicz programowanie par. To najlepszy możliwy przegląd kodu. Każdy inny mechanizm powoduje grę obwiniającą. Programowanie w parach wywołuje ducha zespołu i wspólną odpowiedzialność. Dodatkowo dyskutujesz o swoich pomysłach ze swoją parą, szybko zawodzisz, podejmujesz działania naprawcze i tylko para zakodowana i poddana przeglądowi kod zostaje zatwierdzona w Systemie zarządzania konfiguracją (CMS). Programowanie szczęśliwej pary!

karthiks
źródło
Całkowicie się zgadzam. Programowanie w parach zapewnia sprawdzanie jakości kodu podczas pisania. Ponadto, wprowadź TDD, a przetestujesz kod o kontrolowanej jakości, który zostanie wydany w gałęzi funkcji. Przetestuj funkcje w oddziale z osobno napisanymi testami jakości. Po przejściu połącz z mistrzem. Wyczyść kod, wyczyść mistrza.
dooburt
Programowanie w parach nie działa u bardzo dużego odsetka programistów i zaryzykuję stwierdzenie, że prawdopodobnie wyklucza to najlepszych programistów. Większość programistów SW interesuje się programowaniem komputerowym, ponieważ jest zamknięta w sobie, co oznacza, że ​​wolą pracować z komputerami bardziej niż ludzie. Ja po pierwsze muszę wejść do „strefy”, aby być skutecznym, a to oznacza „nie przeszkadzaj mi”. Zostaw mnie w mojej strefie, a ja wykonam pracę 4 lub 5 innych programistów, a następnie niektórych.
Dunk
2

Jeśli pracujesz nad projektem, w którym wiele osób wniesie wkład do bazy kodu, należy ustalić standard.

W tym momencie, z mojego doświadczenia, najlepiej jest wyznaczyć jedną osobę jako „króla” przeglądu kodu, jeśli zechcesz, i to, co on / ona mówi. W związku z tym, jeśli jeden użytkownik nie przestrzega standardów, król się tym zajmie.

Jako twórca wielokrotnie sprawdzam swój kod, aby był czytelny, rozsądny i wszystko inne. Zwykle używamy javadoc lub podobnego w danych językach, w których kodujemy, i używamy tagu @author, aby przypisać własność funkcjom, klasom itp.

Jeśli funkcja nie jest poprawna, rozmawiamy z właścicielem, to samo z klasą itp.

Chris
źródło
2

W mojej firmie do każdego zadania przypisany jest tester do testowania zadania, a także recenzent kodu do przeglądu kodu.

Jeśli Twój produkt został już wydany i chcesz się upewnić, że nie robisz nic złego (takiego jak wyciek uchwytu lub wyciek pamięci), recenzje kodu są świetną rzeczą. Myślę, że podczas początkowego rozwoju przed wypuszczeniem produktu recenzje kodu mogą być zbyt pracochłonne.

Jeśli w Twoim zespole są wszyscy starsi programiści, ocena użytkowników jest nadal przydatna. Wszyscy czasami popełniają błędy. Jeśli w Twoim zespole są osoby starsze i niektóre osoby starsze, pozwól starszym programistom dokonać przeglądu kodu, ale nadal przejrzyj recenzje kodu dla kodu seniora.

Jedną ważną rzeczą dotyczącą przeglądu kodu jest to, że może wychwytywać błędy, które popełniamy, ale nie zastępuje on testowania.

Brian R. Bondy
źródło
2

Polecam używanie recenzji kodu, jeśli nie zajmujesz się programowaniem w parach.

Nie argumentując za i przeciw parowaniu, ale trudno zakwestionować pozytywne skutki ciągłego sprawdzania kodu przez (przynajmniej) inną osobę. Kod jest nawet napisany i zaprojektowany przez (przynajmniej) dwóch programistów - trudno go poprawić. Mówię „przynajmniej”, ponieważ imo, powinieneś bardzo często zmieniać pary, aby wszyscy mieli szansę na pracę z kodem.

Martin Wickman
źródło
2

Jedną z rzeczy, które staram się robić w recenzjach kodu, w których biorę udział, jest to, że sam po przejrzeniu kodu dokonuję statycznej analizy kodu, używając narzędzi takich jak Findbugs, PMD, JavaNCCP i in., I szukam problemów, które te narzędzia znajdują w obrębie kod do przejrzenia. W szczególności chcę spojrzeć na wszystko, co ma niezwykle wysoki poziom złożoności, prosząc ich o wyjaśnienie, dlaczego ten poziom złożoności jest konieczny lub dlaczego sugerowana luka nie jest ważna.

YMMV

mezmo
źródło
2

Tam, gdzie obecnie pracuję, produkujemy urządzenia sprzętowe i oprogramowanie, które się z nimi łączy, które wchodzą w infrastrukturę krytyczną. W związku z tym bardzo mocno koncentrujemy się na jakości wydania. Używamy wariantu Fagan Inspection i przeprowadzamy formalny proces przeglądu. Posiadamy między innymi certyfikat ISO 9001.

Przemysł infrastruktury krytycznej jest bardzo zainteresowany niezawodnością i powtarzalnością tego samego. :-)

Paul Nathan
źródło
2

W mojej firmie mamy obowiązkowe recenzje kodu dla młodszych programistów oraz dobrowolne recenzje dla seniorów. Nie ma wyznaczonego recenzenta kodu, prośby o recenzję wysyłane są do wszystkich programistów.

Ten system działa dobrze - przeglądy są wykonywane w miarę upływu czasu, a zmiany mogą być przeglądane przez kilka zestawów gałek ocznych.

Doskonałe i bezpłatne narzędzie Board Review działa naprawdę dobrze dla nas i okazało się doskonałym sposobem komunikowania recenzji.

GavinH
źródło
2
dobrowolne recenzje dla seniorów. Pracowałem nad wieloma projektami, w których starsi programiści z pewnością mogliby korzystać z recenzji kodu ...
Michel,
1

W mojej firmie nigdy nie przeprowadzamy formalnych przeglądów kodu przed meldowaniem, chyba że modyfikujemy bardzo krytyczną produkcję i nie mamy czasu na wykonanie naszego standardowego procesu kontroli jakości.

Robimy to, że za każdym razem, gdy wydaje nam się, że przegląd kodu byłby użyteczny, dodajemy komentarz do zmodyfikowanego kodu „// todo: przegląd kodu przez Joe”. Zazwyczaj wybieramy Joe, ponieważ jest on właścicielem zmodyfikowanego kodu, ale jeśli te kryteria wyboru nie mają zastosowania (zwykle tak się dzieje), po prostu losowo wybieramy kogoś innego. I oczywiście, jeśli Joe będzie w tym czasie dostępny, zastosujemy starą dobrą metodę oceny przez ramię.

Jako recenzent jedyne, co musisz zrobić, to okresowo wyszukiwać cały kod pod kątem „// todo: przegląd kodu przeze mnie” , przeglądać zmiany i sprawdzać kod z powrotem bez komentarza „todo ...”

W przypadku problemu przełączamy się z powrotem na tradycyjne metody komunikacji (poczta / czat / itp.).

Ta metoda zapewnia nam dwie główne cechy, których szukamy w systemie recenzji:

  • bardzo lekki nad głową
  • identyfikowalność
Brann
źródło
1

Uważamy, że najbardziej efektywnym sposobem dokonywania przeglądów kodu jest korzystanie z github w celu pokazania różnic w gałęzi za pomocą github.


  • Czy jedną osobę uważa się za strażnika jakości i przegląda kod, czy też zespół jest właścicielem normy?

    • Zależy od wielkości zespołu - zespół 3 osób będzie miał 1 seniora, który najlepiej zna dobre praktyki, podczas gdy zespół 12 osób może mieć 3 lub 4 osoby, które spełnią tę rolę.
  • Czy przeglądasz kod jako ćwiczenie zespołowe przy użyciu projektora?

    • Osobiście. 1 na 1, by być mniej groźnym. Czasami odbywa się to w obszarze wspólnym, aby rozpowszechniać wiedzę. Zależy od dokładnej sytuacji, ludzi, harmonogramu itp.
  • Czy odbywa się to osobiście, pocztą elektroniczną lub za pomocą narzędzia?

    • Osobiście. Używamy git i github i ma świetne narzędzia do przeglądania kodu i porównywania różnic, aby ułatwić przeglądanie kodu.
  • Czy unikasz recenzji i używasz takich rzeczy, jak programowanie parami i zbiorowe posiadanie kodu, aby zapewnić jakość kodu?

    • Staramy się robić obie te rzeczy odpowiednio. Oznacza to, że gdy utkniesz w szczególnie drażliwym problemie, pracujesz nad podstawową infrastrukturą lub przygotowujesz się do urlopu lub opuszczasz firmę, można przeprowadzić parowanie w celu dzielenia się i przekazywania wiedzy. Większość kodu lub kodu, który zawiera coś więcej niż zmiany kosmetyczne, również powinna zostać przejrzana na końcu.

Podobnie jak w przypadku wszystkich pozycji kodujących, właściwa odpowiedź powinna uwzględniać:

  • Rodzaj firmy (np. Startup a duża korporacja)
  • Wielkość spółki
  • Liczba programistów
  • Budżet
  • Ramy czasowe
  • Obciążenie pracą
  • Złożoność kodu
  • Zdolność recenzenta (ów)
  • Dostępność recenzentów
Michael Durrant
źródło
0

Pracowałem w wielu firmach i widziałem wiele procesów. Mój obecny zespół zajmuje się tym, co widziałem najlepiej.

Stosujemy zwinny proces rozwoju, w ramach którego mamy bramy, przez które musi przejść każda historia. Jedną z tych bram jest przegląd kodu. Historia nie przechodzi do testowania, dopóki przegląd kodu nie zostanie zakończony.

Dodatkowo przechowujemy nasz kod na github.com. Więc po tym, jak programista zakończy zmianę, publikuje link do zatwierdzenia w historii.

Następnie oznaczamy innych programistów do oceny. Github ma system komentarzy, który pozwala komentować bezpośrednio na linii kodu, o której mowa. Następnie Github wysyła wiadomość e-mail do dystrybucji, więc czasami rzeczywiście dostrzegamy dodatkowe spojrzenia niektórych innych naszych zespołów.

Ten proces działał dla nas bardzo dobrze. Używamy zwinnego narzędzia procesowego, które ułatwia publikowanie zleceń, a także ułatwia komunikację.

Jeśli problem jest szczególnie trudny, usiądziemy i przedyskutujemy go. Działa to, ponieważ jest integralną częścią naszego procesu i każdy ma wkład w to, jak ten proces działa. Możemy przenieść bilety z powrotem do trwającego, jeśli weryfikacja kodu spowoduje konieczną przeróbkę, a następnie może zostać ponownie sprawdzona po wprowadzeniu zmian lub recenzent może zauważyć w historii, że naprawienie elementów jest wystarczające i nie wymaga ponownej oceny.

Jeśli testowanie coś cofa, wszystko wraca do stanu początkowego, a wszelkie dalsze zmiany są również sprawdzane.

Bill Leeper
źródło