Czy powinniśmy spróbować przejrzeć cały nasz kod?

18

Obecnie modyfikujemy proces programowania i zastanawiam się, czy nie powinniśmy próbować sprawdzać 100% naszych zatwierdzeń.

Jakie jest twoje doświadczenie związane z recenzjami kodu?

  • Czy spędzasz na nich „dużo” czasu (powiedzmy 1/2 godziny dziennie), czy po prostu przeskakujesz przez maksymalnie 5/10 minut?
  • Czy masz określoną ilość czasu na dzień / tydzień / sprint / projekt?
  • Co najważniejsze, czy uważasz, że celem powinno być sprawdzenie 100% kodu do recenzji, czy 100% nie jest konieczne?
Simeon
źródło
Spróbuj dotknąć 80% kodu przy 20% wysiłku. Inwestuj w najlepsze zautomatyzowane narzędzia, które można kupić za pieniądze.
Job
2
Zautomatyzowane narzędzia są świetne, ale stają się bezużyteczne, chyba że masz czas i zasoby, aby utrzymać wszystkie testy jako aktualne.
Kieren Johnstone,

Odpowiedzi:

19

W każdej historii mamy zadanie „Przegląd kodu”. Ktoś, kto idealnie nie byłby zaangażowany w rozwój tej historii, przejrzy wszystkie zmiany kodu związane z tą historią. To dobrze działa.

Dużo czasu? Nie bardzo, zależy od tego, ile kodu - szukamy oczywistych błędów, literówek, podstawowego sprawdzania logiki, nieprzechwyconych wyjątków itp.

Jest to krok jakościowy, który wykrywa błędy, dlatego ma pewną wartość. Przydzielanie czasu może nie być najlepszym sposobem na zrobienie tego - co powiesz na to, że jeśli coś jest dość złożone, powinno zostać poddane przeglądowi kodu?

Nawiasem mówiąc, ważne jest, aby ktoś inny dokonał przeglądu kodu.

Kieren Johnstone
źródło
3
„Nawiasem mówiąc, ważne jest, aby ktoś inny przeglądał kod ..”, czy pytanie oznacza, że ​​ta sama osoba, która pisze kod, powinna go przejrzeć? Jeśli tak to gdzie? Chciałbym to naprawić :)
Simeon
4
Nie, nie, mówiłem wyczerpująco i powiedziałem, że to ważne
Kieren Johnstone,
5
@Simeon to strasznie powszechne błędne przekonanie, że właściciel może przejrzeć własny kod. Podważa to całą operację
Tom Squires
5
@TomSquires: Dokładnie. Kiedy pracujesz z kawałkiem kodu od dłuższego czasu, możesz stać się „ślepy” na inne oczywiste wady w nim, ponieważ postrzegasz go jako taki, jaki powinien być zamiast tego, czym jest. Problemy te będą łatwiejsze do wykrycia dla kogoś, kto nigdy wcześniej nie widział kodu. Pisarze mają ten sam problem i podobnie jak książki nie są wydawane bez korekty, kod nie powinien być wydawany bez recenzji. Istnieją również inne korzyści z robienia przeglądów kodu, na przykład jest dobry do przesyłania wiedzy między członkami twojego zespołu.
hammar
@hammar - oczywiście próba znalezienia kogoś, kto nie napisał kodu, który ma czas na tak dokładne zaznajomienie się z nim, że może on pomóc go przejrzeć, jest wyzwaniem
Peter Ajtai
12

Ważniejszym zagadnieniem niż to, jak wiele kodu obejmuje recenzje, jest skuteczność ich recenzji. Jeśli w twoich recenzjach odkryjesz kilka problemów lub nie będzie ich wcale, osiągnięcie pełnego zasięgu będzie bezużyteczne.

Najpierw popracuj nad tym, aby Twoje recenzje były bardziej skuteczne, a następnie zdecyduj o ich zasięgu.

Recenzje powinny być wykonywane nie tylko na kodzie, ale także na projekcie.



Ponadto recenzje nie zastępują testów i narzędzi:

  • Recenzje mogą kodować na sucho
  • Recenzje mogą obejmować analizę kodu
  • Recenzje sprawdzają ponowne użycie i czytelność
  • Recenzje mogą badać niektóre aspekty wydajności, jednak nie zastępuje to rzeczywistego pomiaru czasu przebiegu i znajdowania szyjek butelek
  • Istnieją narzędzia do analizy kodu statycznego
  • Istnieją narzędzia do testowania konwencji kodowania, nie marnuj na to czasu
  • Testy mokrego testu jednostkowego, systemowego i integracji
  • Testy jednostkowe, systemowe i integracyjne mogą być powtarzane automatycznie, przeglądy kodu są zwykle jednorazowe
  • Testy jednostkowe powinny mieć wysoki zasięg kodu i testować zarówno główne scenariusze sukcesu, jak i warunki końcowe, przeglądy kodu mogą to zrobić tylko częściowo
  • Testy integracyjne sprawdzają zdolność jednostek lub systemów do współpracy, przegląd kodu nie może tego zastąpić
  • Testy systemu testują funkcjonalność całego systemu, przegląd kodu nie może tego zastąpić



Spróbuj poświęcić określoną ilość czasu na miesiąc (lub na sprint) na recenzje. Wybierz kod, który chcesz przejrzeć w następnym dedykowanym gnieździe, używając heurystyki, takiej jak:

  • Kod, który może zawierać niezidentyfikowany błąd, który został zgłoszony
  • W kodzie z tym niedawno wykryto błędy
  • Kod z problemami z wydajnością (szyjki butelki)
  • Kod napisany przez nowych programistów
  • Stary kod, który został niedawno zaktualizowany przez osobę, która wcześniej go nie znała
  • Kod w nowych obszarach
  • Istniejący kod, o którym nowi programiści powinni się dowiedzieć
  • Kod rozwiązujący złożone problemy
  • Kod, który został zidentyfikowany jako złożony przez narzędzia analityczne

I pamiętaj, że przeglądasz kod (lub projekt lub testy), a nie autorzy.



Polecam następujące materiały do ​​czytania:

Selektywne recenzje bez pracy domowej
Najlepsze zachowane tajemnice recenzji kodu partnerskiego

Danny Varod
źródło
5

To zależy.

To zależy od tego, co robi twoje oprogramowanie:

  • Jeśli kontroluje elektroniczny rozrusznik serca lub prom kosmiczny, to zdecydowanie tak.

  • Jeśli jest to prototyp wyrzucany, prawdopodobnie nie.

Zależy to również od tego, jak dobrze dysponujesz zasobami, jak bardzo masz doświadczenie w programowaniu i czego szukasz w recenzjach kodu. (Należy pamiętać, że przeciętny programista sprawdzający czyjś kod prawdopodobnie zauważy problemy ze stylem i ominie subtelne błędy algorytmiczne ... zwłaszcza biorąc pod uwagę, że recenzowanie kodu jest uciążliwe.)

Radzę zaoszczędzić wysiłku związanego z przeglądaniem kodu w przypadku kodu, w którym poprawność ma kluczowe znaczenie, a koszt niewykrytych błędów jest wysoki.

Stephen C.
źródło
5

Najpierw musisz odpowiedzieć na to pytanie: Dlaczego przeglądasz kod?

Dzięki tej odpowiedzi możesz dowiedzieć się, który kod należy przejrzeć.

Niektóre przeglądy kodu pozwalają dokładnie wykonać lub wykonać testowanie. Jeśli taki jest cel twoich recenzji, to zbliżenie się do 100% jest dobrym pomysłem, jeśli masz mało testów. Jednak zezwolenie na to narzędziom testowym zmniejszyłoby potrzebę przeglądu całego kodu.

Wydaje się, że większość dobrych recenzji koncentruje się na dzieleniu się wiedzą i zwiększaniu możliwości programistów w recenzji (zarówno tego, który napisał kod, jak i tych, którzy go przeglądają). Ponieważ jest to główny powód recenzji, upewnienie się, że sprawdzenie 100% kodu jest prawdopodobnie przesadą.

John Fisher
źródło
3

W idealnym świecie wszystko byłoby wyraźnie czytane przez autora i recenzowane przez co najmniej jedną inną osobę, od specyfikacji wymagań, instrukcji użytkownika po przypadki testowe. Ale recenzje, nawet proste kontrole na biurku, wymagają czasu i kosztują pieniądze. Oznacza to, że musisz wybrać, co powinieneś przejrzeć i kiedy powinieneś to sprawdzić.

Zalecam nadanie priorytetu rzeczom do oceny, wybranie sposobu, w jaki chcesz je przejrzeć, i próba przejrzenia jak największej ilości informacji z odpowiednim poziomem szczegółowości. Priorytetyzacja może być oparta na rodzaju artefaktu, na przykład stwierdzeniu, że wymagania muszą zostać poddane przeglądowi, kod projektu i produkcji powinny zostać przejrzane, a przypadki testowe mogą zostać przejrzane. W ramach tego można również określić, że komponenty o wysokim ryzyku lub wartości mają pierwszeństwo w przeglądzie, a może bardziej formalny przegląd.

W miarę upływu czasu wszystko zaczyna się od tego, jak wysoki jest priorytet tego komponentu. Były czasy, w których spędziłem 10-15 minut na recenzowaniu, a innym razem wiele osób przeczytało kod indywidualnie, a następnie poszło do pokoju, aby przeprowadzić bardziej formalny proces inspekcji, który trwa 30-45 minut (w zależności od wielkości moduł).

Ostatecznie jest to równowaga między czasem, kosztami, zakresem i jakością. Nie możesz mieć ich wszystkich, więc musisz zoptymalizować, gdzie możesz.

Thomas Owens
źródło
3

Sugerujemy, aby w ogóle planować jakieś recenzje, mieć kilka wspólnych wytycznych dotyczących zakresu i celu przeglądu, aby upewnić się, że recenzje nie powodują niepotrzebnych konfliktów między członkami zespołu.

Spójne zespoły budują lepsze projekty. Ludzie mogą stracić relacje z powodu bzdur lub żądań doskonałości. Zawsze jest jedna osoba, która narzeka na to lub tamto i dręczy innych tylko dlatego, że taki jest ...

Bez szans
źródło
2

Rezerwuję godzinę dziennie na recenzowanie, ale nie zawsze tego potrzebuję. Nasza baza kodów jest współużytkowana przez kilkadziesiąt produktów. Zgodnie z naszymi zasadami, trywialna zmiana kodu unikalna dla jednego produktu jest w porządku, aby dokonać odprawy bez recenzji. Bardziej złożone zmiany jednego produktu wymagają przeglądu, ale może być tak nieformalne, jak zadzwonienie do swojego kolegi z pracy, aby raz jeszcze to zrobić. Zmiany we wspólnym kodzie wymagają bardziej formalnej weryfikacji, w tym programistów dotyczących innych produktów. Myślę, że nasza polityka zapewnia dość dobrą równowagę w porównaniu do innych firm, w których pracowałem.

Codziennie spędzam więcej czasu na recenzjach niż niektórzy z moich kolegów z mniej ważnymi rolami, ale nie uważam tego za nieuzasadniony czas, ponieważ przed polityką recenzowania mogłem z łatwością zmarnować więcej czasu niż na śledzenie błędów, które programista na innym wprowadzonym produkcie.

Karl Bielefeldt
źródło
Czy to średnia? Ograniczanie złożonej recenzji do godziny wydaje się dziwne, a jeśli nie ma zbyt wiele do przeglądu ... no cóż, nie widzę, jak godzina dziennie byłaby wykonalna?
Kieren Johnstone
To nie jest limit. Ustawiłem czas na podstawie tego, ile czasu to zajęło, a nie na odwrót. Zwykle mogę zrobić 2 recenzje w ciągu godziny. Jeśli to potrwa dłużej, Twoje kontrole są zbyt duże, nie otrzymujesz wystarczającej ilości recenzji projektu lub narzędzia do sprawdzania kodu są zbyt kłopotliwe. Recenzje kodu są czekiem, a nie przeprojektowaniem.
Karl Bielefeldt
2

Zrobiliśmy 100% recenzji kodu. Jest to o wiele tańsze niż testowanie, zwłaszcza testowanie 100% pokrycia kodu. Nie poświęcamy im zbyt wiele czasu, a recenzowanie przez ponad godzinę dziennie staje się mniej wydajne. (30 minut to nie dużo).

Podczas zerowania procesu rób notatki. Co znalazłeś? Co później znalazło QA? Co znaleźli twoi klienci? Dlaczego te błędy ci uciekły?

MSalters
źródło
7
W jaki sposób recenzowanie jest tańsze niż testy automatyczne? Zakładając, że napiszesz test raz, raz przejrzysz test i będziesz mieć stabilny zestaw testów, powinieneś poświęcać znacznie mniej czasu i pieniędzy na wykonywanie testów niż potrzeba na przeprowadzenie jakiejkolwiek oceny (przez cały czas trwania projektu). Ponadto dążenie do zapewnienia 100% pokrycia kodu jest marnotrawstwem zasobów, co może być przyczyną dłuższego czasu / kosztu testowania. Kwestionuję również 30 minut recenzji - możemy przejrzeć moduł przez 30 minut, ale jest czas przygotowania do przeczytania kodu na początku, zrozumienia jego roli w systemie i komentowania go.
Thomas Owens
Roszczenia @MSalters nie są niespotykane, chociaż ja też jestem sceptyczny, ponieważ zajmuje to tylko 30 minut. Czytałem tylko o jednym miejscu, w którym inspekcja była bardziej opłacalna niż zautomatyzowane testowanie jednostkowe i była to NASA. W takim przypadku ostatecznie zrezygnowali z testów jednostkowych, ponieważ tańsza była ręczna kontrola kodu. Oczywiście NASA nadal miała stosunek tester: deweloper 12: 1, więc robili też wiele innych testów ...
Michael
2
@Thomas Owens: testy jednostkowe wykrywają proste błędy. Drogie błędy to te, w których wiele jednostek łączy się w nieoczekiwany sposób. Innym rodzajem błędów są pominięte przypadki narożne. Deweloper, który przegapił przypadek, również nie napisze testu jednostkowego, ale dostrzeże go drugi zestaw oczu.
MSalters,
@MSalters Dlatego piszesz automatyczne testy integracyjne i systemowe, a także testy jednostkowe. Naprawdę jedynymi testami, które mogą wymagać ręcznego wykonania, są testy akceptacyjne. A przeglądanie testów podczas tworzenia pomógłoby zapewnić, że testowane są najbardziej krytyczne przypadki.
Thomas Owens
2

Regularnie przeglądaj kod głównie w celu budowania zespołu i dzielenia się pomysłami na temat implementacji. W ten sposób możesz wiele się nauczyć od współpracowników.

koder
źródło
Oznacza to tylko kilka korzyści. Czy uważasz, że znalezienie błędów jest ważne? Jeśli tak, to ile?
JeffO
Oczywiście znajdowanie błędów jest ważne, ale większą korzyścią jest długoterminowa wiedza uzyskana z przeglądów kodu. Być może błąd został spowodowany złym podejściem, któremu można zapobiec w przyszłości
programista
2

Wymagamy weryfikacji kodu równorzędnego przy każdym zameldowaniu. Jeśli nie ma dostępnego peera, organizujemy kontrolę po odprawie. Recenzent został odnotowany w komentarzu do kontroli źródła.

Nie zajmuje to dużo czasu, a ponieważ są wykonywane między rówieśnikami, nie ma dla nich toksycznego aspektu dla dorosłych i dzieci.

Jim In Texas
źródło
2

Konieczny jest przegląd kodu, IMO. Masz 99,999 ...% czasu nie zawsze będzie w porządku, więc musisz się upewnić, że jest poprawny. Czy mam ustalony czas? Nie. Ale poświęcam czas na sprawdzenie mojego kodu. Zwykle kolega robi to samo.

Dynamiczny
źródło
1

Przeglądy kodu mogą wydawać się zniechęcające, ale są cennym narzędziem, jeśli są właściwie przeprowadzane. Będą Twoją pierwszą linią obrony przed błędami projektowymi i wdrożeniowymi. Jeśli nie przeprowadzasz recenzji kodu dla każdej wprowadzonej funkcji, powinieneś zacząć JAK NAJSZYBCIEJ.

Jeśli chodzi o ilość czasu, którą należy poświęcić na recenzje, dobrą praktyką jest pozostawienie 5–10% całkowitego szacowanego czasu programowania na przeprowadzenie przeglądu kodu i udzielenie odpowiedzi.

Mamy oficjalny dokument na temat przeprowadzania skutecznych recenzji kodu, które mogą pomóc Ci zacząć od właściwej stopy. Jest to przewodnik krok po kroku omawiający typowe pułapki, z którymi możesz się spotkać oraz sposoby ich rozwiązywania. Możesz pobrać go z naszej strony internetowej.

Katie B.
źródło