Dobre wytyczne i praktyki dotyczące obowiązkowego przeglądu kodu [zamknięte]

Próbujemy obowiązkowej weryfikacji kodu przy każdym zatwierdzeniu - nic nie dostaje się do wzorca, co nie zostało zatwierdzone przez co najmniej 1 osobę niebędącą autorem - dla kilku sprintów. Mamy wpisowe od deweloperów i kadry zarządzającej (co jest niesamowitą sytuacją) i chcemy uzyskać niektóre z korzyści, z których jest znany:

• oczywista redukcja błędów
• większa świadomość zmian zachodzących wokół projektu
• „Wiem, że ktoś na to spojrzy, więc nie będę leniwy” / efekt anty-kowbojski
• zwiększona spójność w ramach / pomiędzy projektami

Ale wprowadzamy coś, co jest znane ze zmniejszania prędkości, a jeśli zrobione źle, może stworzyć głupi biurokratyczny krok w procesie zatwierdzania, który nie zajmuje nic, tylko zajmuje czas. Rzeczy, które mnie niepokoją:

• recenzje przekształcają się w zbieranie nitów
• (hiperbolicznie) ludzie otwierają ogromne problemy architektoniczne w ramach przeglądu zatwierdzania dwóch linii.
• Nie chcę uprzedzać odpowiedzi w innych sprawach.

Chociaż wszyscy jesteśmy rozsądnymi ludźmi i będziemy przeprowadzać wiele samoanalizy, zdecydowanie możemy skorzystać z pewnego wygranego w bitwie wglądu w to, jakie rzeczy powinniśmy starać się osiągnąć w sesji przeglądowej, aby recenzje naprawdę działały dla nas . Jakie wytyczne i zasady okazały się skuteczne?

1. Skróć recenzje.

   Trudno jest skoncentrować się, szczególnie podczas przeglądu kodu, przez długi czas. Co więcej, recenzje długiego kodu mogą wskazywać, że jest zbyt wiele do powiedzenia na temat kodu (patrz dwa następne punkty) lub że recenzja staje się dyskusją na temat większych zagadnień, takich jak architektura.

   Recenzja powinna pozostać recenzją, a nie dyskusją. Nie oznacza to, że autor kodu nie może odpowiedzieć, ale nie powinien przerodzić się w długą wymianę opinii.

2. Unikaj sprawdzania kodu, który jest zbyt zły.

   Recenzowanie kodu niskiej jakości jest przygnębiające zarówno dla recenzenta, jak i autora kodu. Jeśli fragment kodu jest okropny, przegląd kodu nie jest przydatny. Zamiast tego należy poprosić autora o poprawne przepisanie kodu.

3. Użyj automatycznych kontrolerów przed przeglądem.

   Zautomatyzowane mechanizmy kontrolne unikają marnowania cennego czasu na błędy wskazujące, które można znaleźć automatycznie. Na przykład w przypadku kodu C # uruchomienie StyleCop, metryk kodu, a zwłaszcza analizy kodu, jest dobrą okazją do znalezienia niektórych błędów przed przeglądem. Następnie przegląd kodu można wydać na punkty, które są bardzo trudne do zrobienia dla maszyny.

4. Wybierz starannie osoby dokonujące recenzji.

   Dwie osoby, które nie znoszą się nawzajem, nie dokonają dobrego przeglądu jednego z kodów drugiego. Ten sam problem powstaje, gdy jedna osoba nie szanuje drugiej (nawiasem mówiąc, czy to recenzent, czy autor).

   Ponadto niektóre osoby po prostu nie mogą zobaczyć swojego kodu, dlatego wymagają specjalnego przeszkolenia i przygotowania, aby zrozumieć, że nie są krytykowane i nie powinny postrzegać go jako coś negatywnego. Wykonanie recenzji, nieprzygotowanej, nie pomoże, ponieważ zawsze będą w defensywie i nie będą słuchać krytyków swojego kodu (przyjmując każdą sugestię jako krytykę).

5. Dokonuj zarówno nieformalnych, jak i formalnych recenzji.

   Posiadanie listy kontrolnej pomaga skoncentrować się na dokładnym zestawie wad, unikając przejścia w zbieranie nitów. Ta lista kontrolna może zawierać takie punkty, jak:

   • Zastrzyk SQL,
   • Błędne założenia dotyczące języka, który może prowadzić do błędów,
   • Określone sytuacje, które mogą prowadzić do błędów, takie jak pierwszeństwo operatora. Na przykład w języku C # var a = b ?? 0 + c ?? 0;może wyglądać dobrze dla kogoś, kto chce dodać dwie zerowalne liczby z koalescencją na zero, ale tak nie jest.
   • Dealokacja pamięci,
   • Leniwe ładowanie (z dwoma ryzykami: ładowanie tego samego więcej niż jeden raz i wcale nie ładowanie go),
   • Przelewy,
   • Struktury danych (na przykład z błędami, takimi jak prosta lista zamiast zestawu skrótów),
   • Walidacja danych wejściowych i ogólnie programowanie obronne,
   • Bezpieczeństwo wątków,
   • itp.

   Zatrzymuję tę listę tutaj, ale istnieją setki punktów, które mogą znaleźć się na liście kontrolnej, w zależności od słabych punktów konkretnego autora.

6. Stopniowo dostosowuj listę kontrolną.

   Aby zachować konstruktywność i użyteczność w miarę upływu czasu, listy kontrolne stosowane w formalnych przeglądach powinny być z czasem dostosowywane w zależności od wykrytych błędów. Na przykład pierwsze nieformalne przeglądy mogą ujawnić pewne ryzyko związane z iniekcją SQL. Sprawdzanie wtrysku SQL zostanie uwzględnione na liście kontrolnej. Kiedy kilka miesięcy później wydaje się, że autor bardzo ostrożnie podchodzi do zapytań dynamicznych vs. sparametryzowanych, SQL Injection może zostać usunięty z listy kontrolnej.

Mamy prawie jak listę kontrolną:

• Pokaż mi opis zadania.
• Przeprowadź mnie przez wynik i pokaż, że działa. Uruchom różne scenariusze (nieprawidłowe dane wejściowe itp.).
• Pokaż mi zaliczone testy. Jak wygląda zasięg testu?
• Pokaż mi kod - właśnie tam szukamy oczywistej nieefektywności.

Działa dość dobrze.

Myślę, że wystarczy osoba, która ma władzę nad innymi, administrator lub moderator, aby wyciąć niepotrzebne komentarze i przyspieszyć recenzowanie rzeczy, które wymagają szybkiej oceny. Pojedynczy decydent.

Minusem tego byłoby to, że ta osoba musi to zrobić jako główne zadanie, podczas gdy on mógłby robić coś innego, i prawdopodobnie chciałbyś mieć najbardziej doświadczoną osobę na tym stanowisku.

Drugą rzeczą jest automatyzacja jak najwięcej!

• kontrolowanie białych przestrzeni
• oprogramowanie do kontroli stylu
• automatyczne kompilacje przed przeglądem kodu
• automatyczne testowanie przed przeglądem kodu

Te rzeczy usuną przynajmniej niektóre rzeczy, które ludzie mogą komentować bez prawdziwej potrzeby. Jeśli nie buduje się lub ma spacje końcowe, nie jest wystarczająco dobry do recenzji, napraw go i ponownie ubiegaj się o recenzję. Jeśli nie buduje się lub jakiś test kończy się niepowodzeniem, jest oczywiste, że nie jest wystarczająco dobry.

Wiele zależy od twoich technologii, ale im więcej możesz sprawdzić, tym lepiej.

Nie wygraliśmy jeszcze tej bitwy, ale to nam się przydało.