Uzyskiwanie WSZYSTKICH programistów dokonuje przeglądu kodu

13

Jestem programistą w zespole programistów 7-8. Od dłuższego czasu robimy recenzje kodu, a jakość kodu poprawiła się z czasem.

Jednak ostatnio zauważyłem, że niektórzy programiści są proszeni o więcej recenzji kodu niż inni. Obawiam się, że to ze względu na ich elastyczne podejście.

Moim zdaniem nie w ten sposób należy przeprowadzać recenzje kodu: cały zespół powinien być za to odpowiedzialny, a recenzentów kodu nie należy wybierać ze względu na ich gotowość do łatwego akceptowania zmian.

Jak radzisz sobie z tym problemem w swoim zespole?

Czy ustaliłeś zasady wyboru recenzentów kodu?

Czy uważasz, że recenzenci kodu powinni być nagradzani za czas poświęcony na (dobre) recenzje kodu? A jak powinny być nagradzani?

Dziękuję za twoje odpowiedzi / pomysły.

guillaumegallois
źródło
7
Czy zastanawiałeś się nad stworzeniem okrągłego systemu robin, w którym zarówno programista pozostawiony jest w ciemności na temat tego, kto recenzuje, a recenzent pozostaje w ciemności na temat tego, kto jest programistą?
Neil
1
Nie mam, ale podoba mi się ten pomysł! Dzięki!
guillaumegallois
1
Kto jest odpowiedzialny i dlaczego nie wykonuje swojej pracy, co powinno obejmować upewnianie się, że wszyscy inni wykonują swoją pracę?
JeffO
W moim zespole recenzenci są automatycznie przypisywani przy każdym otwarciu żądania ściągnięcia. Recenzenci są wybierani z rundy zespołu. Dla każdego z naszych repozytoriów mamy hak internetowy, który automatycznie przypisuje recenzentów przy każdym otwarciu PR. Zasadniczo przechowuje listę wszystkich programistów i tych, którzy zostali ostatnio przydzieleni, i po prostu przegląda listę.
Dan Jones

Odpowiedzi:

12

Nie wybieramy recenzentów.

W naszym zespole:

  • Wszystkie zmiany kodu muszą zostać sprawdzone przed zakończeniem żądania ściągnięcia
  • Co najmniej jeden programista musi dokonać przeglądu kodu (dwóch lub więcej recenzentów jest lepszych, a niezwykle pomocna jest również obecność testerów, analityków i innych członków zespołu wykonujących przegląd kodu)

Recenzje kodu nie są przypisywane, ludzie je odbierają, gdy dla nich działa. Rozumiemy, że nie możemy przepchnąć historii przez rurociąg. Czasami ktoś wspomina, że ​​czeka na CR w pojedynku, ale o to chodzi.

Podoba mi się ten model, pozwala ludziom podnieść to, co mogą i unika „dawania ludziom pracy”.

Liath
źródło
6

Wprowadź zasadę, że błąd można przypisać do naprawy, nie tylko tym, którzy dokonali zmiany, ale tylko tym, którzy ją sprawdzili. To powinno stworzyć poprawną perspektywę dla przeglądu kodu.

Jeśli chodzi o,

Czy uważasz, że recenzenci kodu powinni być nagradzani za czas poświęcony na (dobre) recenzje kodu?

Nie jestem pewien, jak ogólnie programiści są wynagradzani za wykonywanie swojej pracy, oprócz tego, że otrzymują wynagrodzenie i są dumni z tego, co zrobili. Ale ponieważ recenzowanie kodu jest częścią ich pracy, recenzent powinien znaleźć czas na recenzję i w jakiś sposób udostępnić kredyt.

max630
źródło
2
To ciekawy pomysł, ale wiele razy, gdy pojawia się błąd, 90% pracy polega na ustaleniu, co jest przyczyną błędu, a 10% pracy polega na jego naprawie. Wykonywanie sekcji zwłok, aby dowiedzieć się dokładnie, jakie zmiany zostały wprowadzone, może się nawet nie zdarzyć, chyba że pomoże to ustalić, co się dzieje lub jak zrobić bezpieczną poprawkę.
DaveG
Podkreśliłeś, że recenzenci kodu powinni otrzymać uznanie. Jest to zdecydowanie problem, którym należy się zająć. Dziękuję za odpowiedź!
guillaumegallois
Myślę, że może to zmusić ludzi, by nie próbowali w ogóle przeglądać kodu lub być może nie wykonasz żadnej pracy, ponieważ ludzie zaczną nitpicking.
Mateusz
5
Ta odpowiedź zakłada, że ​​celem recenzji kodu jest znalezienie błędów. Tak nie jest, głównym celem jest utrzymanie kodu w możliwym do utrzymania i ewoluowalnym stanie (a jeśli masz szczęście, znajdziesz również pewne błędy).
Doc Brown
@DocBrown, aby zapobiec błędom, a także upewnić się, że poprawienie błędu w przyszłości będzie szybsze (zarówno poprzez zaznajomienie drugiego programisty z kodem, jak i upewnienie się, że kod jest dobrze napisany)
max630
4

Główny problem, który masz, nie jest techniczny, ale niektóre narzędzia mogą zmniejszyć wysiłek związany z recenzjami kodu. Do pokonania jest kilka wyzwań:

  • Zrozumienie, na czym polega zmiana. Żądania Git Pull dla gałęzi funkcji naprawdę pomagają w tym procesie.
  • Sprawienie, by kod sprawdził wydarzenie, w którym ludzie muszą znajdować się w tym samym pokoju. Zwiększa to stres związany z planowaniem, spotkaniami, zasobami itp. GitHub, GitLab i BitBucket pozwalają na asynchroniczne recenzje, dzięki czemu mogą się zdarzyć, gdy peer jest gotowy.
  • Możliwość przekazywania znaczących informacji zwrotnych podczas przeglądania kodu. Szczerze mówiąc, funkcja komentowania linia po linii w GitHub, GitLab, prośbach o ściągnięcie Bitbucket jest naprawdę bardziej przydatna niż spotkanie twarzą w twarz. To mniej polityczne.

Nie oznacza to, że nie możesz użyć SubVersion lub innych narzędzi (takich jak Rybie Oko), aby pomóc, ale integracja wbudowana w potok Git z gałęziami funkcji naprawdę sprawia, że ​​praca jest mniej uciążliwa.

Poza narzędziami musisz spojrzeć na więcej wyzwań społecznych:

  • Niech programiści rozpoczną dzień od przejrzenia wszelkich otwartych żądań ściągnięcia, aby się na nim podpisać.
  • Poproś programistów o sprawdzenie wszystkich otwartych żądań ściągnięcia przed rozpoczęciem nowego zadania
  • Wymagaj zgody od więcej niż jednej osoby na twoje żądania ściągania.

Warto również sprawdzić, jakie typy zadań są sprawdzane przez bardziej zaangażowane osoby. Być może zbierają wszystkie łatwe recenzje, co sprawia, że ​​wszystko jest bardziej bolesne dla wszystkich innych.

Berin Loritsch
źródło
Ostatni punkt jest dobry. Kiedyś współpracowałem z programistą w małym zespole, który tylko raz sprawdzał zmiany w oprogramowaniu, które napisał, co powodowało znaczne wąskie gardła w innych częściach zespołu.
Robbie Dee,
W takim przypadku wygląda na to, że ktoś próbuje chronić swoje „terytorium”. Na tyle, na ile to możliwe, chcesz promować poczucie własności społeczności dla kodu. Innymi słowy, w kodzie nie ma chronionych świątyń, których nie mogliby dotknąć inni programiści oprócz „błogosławionych”. Rozumiem, że istnieje luka w specjalizacji i nie możesz przejrzeć matematyki, ale możesz przynajmniej sprawdzić, jak dobrze kod odpowiada jego celom.
Berin Loritsch
2

Dobrym pomysłem jest zrobienie tego na zasadzie okrągłego robota - wybierasz kogoś, kto dokonał najmniejszej liczby recenzji twojego kodu. Z czasem każdy w zespole powinien był wykonać mniej więcej taką samą liczbę recenzji. To także rozpowszechnia wiedzę.

Oczywiście będą sporadyczne wyjątki, takie jak święta, gdzie będą szczyty i doliny.

Nie powinno być rozróżnienia między juniorami a seniorami / potencjalnymi klientami. Przegląd kodu należy przeprowadzić dla każdego kodu - bez względu na to, jak są starsze. Zmniejsza tarcie i pomaga dzielić różne podejścia.

Jeśli mimo wszystko nadal martwisz się jakością recenzji, zastanów się nad wprowadzeniem zestawu minimalnych standardów dotyczących pozytywnego wyniku przeglądu kodu. To, co dołączasz, zależy wyłącznie od Ciebie, ale niektóre rzeczy, które warto rozważyć, to pokrycie kodu, testy jednostkowe, usunięcie skomentowanego kodu, metryki, wystarczająca ilość komentarzy, jakość kompilacji, zasady SOLID, DRY, KISS itp.

Jeśli chodzi o zachęcające recenzje kodów, kod jakości jest nagrodą. Wszyscy jesteśmy pewni, że pracowałem nad nieoptymalnymi bazami kodu, w których ból mógł zostać znacznie zmniejszony, gdyby inny programista dał mu kod od samego początku i zaproponował konstruktywne zmiany.

Robbie Dee
źródło
0

Wygląda na to, że zespołowi brakuje formalnego procesu recenzji kodu.

Nie mówię o tworzeniu 350-stronicowego dokumentu Word, ale tylko kilka prostych słów na temat tego, co pociąga za sobą ten proces.

Ważne bity:

  1. Zdefiniuj podstawowy zestaw recenzentów. Brak ogólnych oświadczeń. Nazwij ludzi.

    To powinni być twoi starsi programiści.

  2. Wymagaj, aby więcej niż 1 główny recenzent podpisał się na recenzji.

  3. Zidentyfikuj co najmniej 1 innego recenzenta spoza rdzenia w każdym sprincie lub zwolnieniu, który jest tymczasowym recenzentem podstawowym. Wymagaj, aby w tym czasie podpisywali się na wszystkich recenzjach kodu.

Punkt 3 pozwala innym programistom na przejście do głównej grupy recenzentów. Kilka tygodni spędzą więcej czasu na recenzjach niż inni. Jest to działanie równoważące.

Co do nagradzania ludzi? Wielokrotnie uznanie wysiłku, jaki osoba podejmuje podczas przeglądu kodu przed całym zespołem, może działać, ale nie stresuj się tym.

W razie wątpliwości zdefiniuj proces i powiedz zespołowi, że muszą się go trzymać.

I jest ostatnia rzecz, którą możesz wypróbować - być może kontrowersyjna: niech @ # $% uderzy w wentylator, jeśli mogę użyć idiomu.

Niech zespół zawiedzie, ponieważ proces sprawdzania kodu nie jest przestrzegany. Zaangażuje się zarządzanie, a potem ludzie się zmienią. To naprawdę dobry pomysł w najbardziej ekstremalnych przypadkach, gdy już zdefiniowałeś proces, a zespół odmówił jego przestrzegania. Jeśli nie masz uprawnień do zwalniania ludzi lub dyscyplinowania ich (jak większość wiodących programistów tego nie robi ), musisz zaangażować kogoś, kto może to zrobić.

I nie ma nic takiego jak niemożność zmiany rzeczy. Wbrew temu, co ludzie mogą powiedzieć, to można sterować Titanica - ale nie zanim trafi Burg lodową.

Czasami musisz po prostu pozwolić Titanicowi trafić w lodowy burg.

Greg Burghardt
źródło