Tradycyjnie sprawdzaliśmy kod przed zatwierdzeniem, miałem dzisiaj kłótnię z moim kolegą, który wolał przegląd kodu po zatwierdzeniu.
Po pierwsze, oto trochę tła,
- Mamy doświadczonych programistów, a także nowych pracowników z niemal zerowym doświadczeniem w programowaniu.
- Chcielibyśmy wykonać szybkie i krótkie iteracje, aby wypuścić nasz produkt.
- Wszyscy członkowie zespołu znajdują się w tej samej witrynie.
Zalety recenzji kodu przed zatwierdzeniem nauczyłem się:
- Mentor nowych pracowników
- Staraj się zapobiegać błędom, awariom, złym projektom na wczesnym etapie cyklu programowania
- Ucz się od innych
- Kopia zapasowa wiedzy, jeśli ktoś zrezygnuje
Ale miałem też złe doświadczenia:
- Niska wydajność, niektóre zmiany mogą być przeglądane w ciągu kilku dni
- Trudno zrównoważyć szybkość i jakość, szczególnie dla początkujących
- Jeden członek zespołu poczuł nieufność
Jeśli chodzi o przegląd po zatwierdzeniu, niewiele o tym wiem, ale najbardziej martwię się o ryzyko utraty kontroli z powodu braku przeglądu. Jakieś opinie?
AKTUALIZACJA:
- Używamy Perforce do VCS
- Kodujemy i zatwierdzamy w tych samych gałęziach (gałęzie naprawiające pień lub błędy)
- Aby poprawić wydajność, próbowaliśmy podzielić kod na małe zmiany. Próbowaliśmy też przeglądu dialogów na żywo, ale nie wszyscy przestrzegali tej zasady. To jednak kolejny problem.
code-reviews
piąta
źródło
źródło
Odpowiedzi:
Jak wspomina Simon Whitehead w swoim komentarzu , zależy to od twojej strategii rozgałęziania.
Jeśli programiści mają swój prywatny oddział do programowania (co i tak zaleciłbym w większości sytuacji), przeprowadziłbym przegląd kodu przed połączeniem z bagażnikiem lub głównym repozytorium. Umożliwi to programistom swobodę meldowania się tak często, jak chcą podczas cyklu programowania / testowania, ale za każdym razem, gdy kod przechodzi do gałęzi zawierającej dostarczony kod, jest on sprawdzany.
Ogólnie rzecz biorąc, twoje złe doświadczenia z recenzjami kodu brzmią bardziej jak problem z procesem recenzowania, który ma rozwiązania. Przeglądając kod w mniejszych, pojedynczych porcjach, możesz mieć pewność, że nie zajmie to zbyt długo. Dobrą liczbą jest to, że 150 godzin kodu można przejrzeć w ciągu godziny, ale szybkość będzie wolniejsza dla osób niezaznajomionych z językiem programowania, tworzonym systemem lub krytycznością systemu (krytyczny dla bezpieczeństwa wymaga więcej czasu) - informacje te mogą być przydatne do poprawy wydajności i podjęcia decyzji, kto uczestniczy w przeglądach kodu.
źródło
Jest to mantra, że nikt nie wydaje się być jeszcze cytowany: Przyjazd wcześnie i często :
Pracowałem dla kilku firm, które miały różne podejście do tego. Można na to pozwolić, o ile nie przeszkadza to w kompilacji. Drugi wpadłby w szał, gdyby w ogóle zarejestrowałeś jakieś błędy. Ten pierwszy jest bardzo preferowany. Powinieneś rozwijać się w środowisku, które pozwoliłoby ci współpracować z innymi ludźmi nad rzeczami, które wciąż trwają, przy założeniu, że wszystko to zostanie przetestowane później.
Jest też wypowiedź Jeffa Atwooda: Nie bój się niszczyć :
Dodałbym również, że jeśli ktoś chce, żebyś coś zmienił, w przypadku recenzji, posiadanie historii oryginalnej wersji w źródle jest bardzo cennym narzędziem do nauki.
źródło
Niedawno zacząłem robić recenzje przed zatwierdzeniem w projekcie, w którym jestem i muszę powiedzieć, że jestem mile zaskoczony tym, jak bezproblemowe jest.
Największą wadą przeglądów po zatwierdzeniu, którą widzę, jest to, że często jest to sprawa tylko dla jednej osoby: ktoś sprawdza kod pod kątem poprawności, ale autor zwykle nie jest zaangażowany, chyba że wystąpi poważny błąd. Drobne problemy, sugestie lub wskazówki zwykle nie docierają do autora.
Zmienia to również postrzegany wynik recenzji kodu: jest postrzegany jako coś, co powoduje jedynie dodatkową pracę, w przeciwieństwie do czegoś, w którym każdy (recenzent i autor kodu) może uczyć się nowych rzeczy za każdym razem.
źródło
Recenzje kodu przed zatwierdzeniem wydają się tak naturalne, że nigdy nie przyszło mi do głowy, że recenzje można celowo wykonać po zatwierdzeniu. Z perspektywy ciągłej integracji chcesz zatwierdzić swój kod po zakończeniu, a nie w stanie roboczym, ale być może źle napisanym, prawda?
Może to dlatego, że sposób, w jaki zawsze robiliśmy to w moich zespołach, to dialog na żywo inicjowany przez pierwotnego programistę, a nie asynchroniczne, kontrolowane recenzje oparte na dokumentach.
źródło
Większość repozytoriów obsługuje obecnie zatwierdzanie dwufazowe lub zestaw półek (oddział prywatny, żądanie ściągnięcia, przesłanie łaty lub cokolwiek, jak chcesz to nazwać), co pozwoli ci sprawdzić / przejrzeć pracę przed wciągnięciem jej do głównej linii. Powiedziałbym, że wykorzystanie tych narzędzi pozwoliłoby ci zawsze dokonywać przeglądu przed zatwierdzeniem.
Można również rozważyć kodowanie par (pary seniorów z junior) jako inny sposób zapewnienia wbudowanego przeglądu kodu. Rozważ to jako kontrolę jakości na linii montażowej zamiast po zjechaniu samochodu.
źródło
Zrób jedno i drugie :
źródło
Wszelkie formalne przeglądy powinny być przeprowadzane na plikach źródłowych, które są pod kontrolą konfiguracji, a zapisy przeglądu wyraźnie stwierdzają poprawienie pliku.
Pozwala to uniknąć argumentów typu „nie masz najnowszego pliku” i zapewnia, że wszyscy sprawdzają tę samą kopię kodu źródłowego.
Oznacza to również, że w razie konieczności wprowadzenia korekt po przeglądzie historię można opatrzyć tym faktem.
źródło
W przypadku samego przeglądu kodu głosuję za zatwierdzeniem.
System taki jak gerrit lub koniczyna (myślę) może wprowadzić zmianę, a następnie poprosić recenzenta o zatwierdzenie kontroli źródła (push git), jeśli jest dobra. To najlepsze z obu światów.
Jeśli to nie jest praktyczne, myślę, że po zatwierdzeniu jest najlepszym kompromisem. Jeśli projekt jest dobry, tylko najbardziej młodsi programiści powinni mieć rzeczy wystarczająco złe, abyś nigdy nie chciał ich popełniać. (Dokonaj dla nich przeglądu przed zatwierdzeniem).
Co prowadzi do przeglądu projektu - chociaż możesz to zrobić w czasie przeglądu kodu (lub w tym przypadku w czasie wdrażania klienta), znalezienie problemów projektowych powinno zostać wykonane wcześniej - zanim kod zostanie napisany.
źródło
Dzięki wzajemnej ocenie ryzyko utraty kontroli jest minimalne. Cały czas dwie osoby mają wiedzę na temat tego samego kodu. Muszą się od czasu do czasu przełączać, więc muszą uważnie śledzić kod.
Sensowne jest, aby umiejętny programista i nowicjusz pracowali razem. Na pierwszy rzut oka wydaje się to nieefektywne, ale tak nie jest. W rzeczywistości jest mniej błędów i ich naprawienie zajmuje mniej czasu. Poza tym nowicjusze nauczą się znacznie szybciej.
Aby zapobiec złemu projektowaniu, należy to zrobić przed kodowaniem. Jeśli pojawi się jakaś znacząca zmiana / ulepszenie / nowy projekt, należy to sprawdzić przed rozpoczęciem kodowania. Kiedy projekt jest całkowicie rozwinięty, niewiele pozostaje do zrobienia. Przejrzenie kodu będzie łatwiejsze i zajmie mniej czasu.
Zgadzam się, że przeglądanie kodu przed zatwierdzeniem nie jest konieczne tylko wtedy, gdy kod jest tworzony przez doświadczonego programistę, który już udowodnił swoje umiejętności. Ale jeśli jest nowicjusz, kod powinien zostać sprawdzony przed zatwierdzeniem: recenzent powinien usiąść obok programisty i wyjaśnić każdą dokonaną przez niego zmianę lub ulepszenie.
źródło
Recenzje korzystają zarówno z wcześniejszych, jak i późniejszych zobowiązań.
Zatwierdź przed przeglądem
Brak uruchomionych zatwierdzeń podczas przeglądu
Korzystałem z narzędzi Atlassian i podczas przeglądu zauważyłem, że uruchomiono zatwierdzenia. Jest to mylące dla recenzentów, więc odradzam.
Rewizje po przeglądzie
Po tym, jak recenzenci uzupełnią swoją opinię, ustnie lub na piśmie, moderator powinien upewnić się, że autor wprowadzi wymagane zmiany. Czasami recenzenci lub autor mogą nie zgodzić się co do tego, czy wyznaczyć pozycję recenzji jako wadę, sugestię lub dochodzenie. Aby rozwiązać spory i upewnić się, że elementy dochodzenia zostały poprawnie usunięte, zespół oceniający zależy od oceny moderatora.
Moje doświadczenie z około 100 inspekcjami kodu polega na tym, że gdy recenzenci mogą odwoływać się do jednoznacznego standardu kodowania, a dla większości rodzajów błędów logicznych i innych błędów programistycznych wyniki przeglądu są ogólnie jasne. Czasami toczy się debata na temat zbierania nitów lub kwestia stylu może przerodzić się w dyskusję. Jednak przekazanie moderatorowi uprawnień decyzyjnych pozwala uniknąć impasu.
Zobowiązanie po przeglądzie
źródło
To zależy od składu zespołu. Dla stosunkowo doświadczonego zespołu, który jest dobry w kwestii małych, częstych zatwierdzeń, a następnie przeglądu po zatwierdzeniu, aby uzyskać drugą parę uwag na temat kodu, jest w porządku. W przypadku większych, bardziej złożonych zatwierdzeń i / lub mniej doświadczonych programistów, dokonaj wstępnego zatwierdzenia recenzji, aby naprawić problemy, zanim się pojawią.
W związku z tym dobry proces CI i / lub bramkowane odprawy zmniejszają potrzebę recenzji przed zatwierdzeniem (i prawdopodobnie po zatwierdzeniu dla wielu z nich).
źródło
Ja i moi koledzy ostatnio przeprowadziliśmy badania naukowe na ten temat, dlatego chciałbym dodać trochę naszych spostrzeżeń, mimo że jest to dość stare pytanie. Zbudowaliśmy model symulacyjny zwinnego procesu / zespołu programistycznego Kanban i porównaliśmy weryfikację przed zatwierdzeniem i po zatwierdzeniu dla wielu różnych sytuacji (różna liczba członków zespołu, różne poziomy umiejętności, ...). Przyjrzeliśmy się wynikom po 3 latach (symulowanego) czasu opracowywania i szukaliśmy różnic w odniesieniu do wydajności (ukończone punkty historii), jakości (błędy znalezione przez klientów) i czasu cyklu (czas od rozpoczęcia do dostarczenia historii użytkownika) . Nasze ustalenia są następujące:
Na tej podstawie wyprowadziliśmy następujące reguły heurystyczne:
Pełny dokument badawczy jest dostępny tutaj: http://dx.doi.org/10.1145/2904354.2904362 lub na mojej stronie internetowej: http://tobias-baum.de
źródło
Moim zdaniem peer review kodu działa najlepiej, jeśli jest wykonywany po zatwierdzeniu.
Polecam dostosowanie strategii rozgałęziania. Korzystanie z gałęzi programisty lub gałęzi funkcji ma wiele zalet ... z których przynajmniej ułatwienie przeglądów kodu po zatwierdzeniu.
Narzędzie takie jak Crucible usprawni i zautomatyzuje proces recenzji. Możesz wybrać jeden lub więcej zatwierdzonych zestawów zmian do uwzględnienia w recenzji. Tygiel pokaże, które pliki zostały dotknięte w wybranych zestawach zmian, śledzi, które pliki każdy recenzent już przeczytał (pokazując% ukończenia ogółem) i pozwoli recenzentom na łatwe komentowanie.
http://www.atlassian.com/software/crucible/overview
Niektóre inne zalety gałęzi użytkowników / funkcji:
Dla niedoświadczonych programistów regularne konsultacje z mentorem i / lub programowanie w parach to dobry pomysł, ale nie uważałbym tego za „przegląd kodu”.
źródło
Obie. (Rodzaj.)
Powinieneś przejrzeć swój kod krótko przed jego zatwierdzeniem. W Git uważam, że strefa inscenizacji jest świetna. Po wprowadzeniu zmian wprowadzam zmiany,
git diff --cached
aby zobaczyć wszystko, co zostało wprowadzone. Używam tego jako okazji, aby upewnić się, że nie sprawdzam żadnych plików, które nie należą (kompilacje, dzienniki itp.) I upewniam się, że nie mam tam żadnego kodu debugowania ani żadnego ważnego kodu skomentowanego na zewnątrz. (Jeśli robię coś, o czym wiem, że nie chcę się zameldować, zwykle zostawiam komentarz na wszystkich literach, aby rozpoznać go podczas inscenizacji).Powiedziawszy to, przegląd kodu równorzędnego powinien być generalnie przeprowadzany po zatwierdzeniu, zakładając, że pracujesz nad gałęzią tematów. Jest to najprostszy sposób, aby upewnić się, że wszyscy sprawdzają poprawność, a jeśli występują poważne problemy, nie ma problemu, aby je naprawić w oddziale lub usunąć je i zacząć od nowa. Jeśli przeprowadzasz recenzje kodu asynchronicznie (np. Używając Google Code lub Atlassian Crucible), możesz łatwo przełączać gałęzie i pracować nad czymś innym bez konieczności osobnego śledzenia wszystkich różnych łat / różnic, które są obecnie sprawdzane przez kilka dni.
Jeśli nie pracujesz nad gałęzią tematów, powinieneś . Redukuje stres i kłopoty oraz sprawia, że planowanie wydania jest mniej stresujące i skomplikowane.
Edycja: Powinienem również dodać, że po testowaniu powinieneś przeglądać kod, co jest kolejnym argumentem przemawiającym za pierwszym użyciem kodu. Nie chcesz, aby twoja grupa testowa bawiła się dziesiątkami łatek / różnic od wszystkich programistów, a następnie zgłaszała błędy tylko dlatego, że zastosowali niewłaściwą łatkę w niewłaściwym miejscu.
źródło
Programowanie w 100% sparowane (bez względu na to, jak starszy jesteś) z wieloma małymi zatwierdzeniami i systemem CI opartym na KAŻDYM zatwierdzeniu (z automatycznymi testami obejmującymi jednostki, integrację i funkcjonalność tam, gdzie to możliwe). Recenzje po zatwierdzeniu dużych lub ryzykownych zmian. Jeśli potrzebujesz jakiejś kontroli bramkowej / przed zatwierdzeniem, Gerrit działa.
źródło
Zaletą sprawdzania kodu przy odprawie (sprawdzanie znajomych) jest natychmiastowa informacja zwrotna, zanim zostaną ukończone duże fragmenty kodu.
Wadą sprawdzania kodu przy odprawie jest to, że może on zniechęcić ludzi do odprawy, dopóki długie odcinki kodu nie zostaną zakończone. Jeśli tak się stanie, całkowicie neguje przewagę.
Trudniejsze jest to, że nie każdy programista jest taki sam. Proste rozwiązania nie działają dla wszystkich programistów . Proste rozwiązania to:
Programowanie z parami obowiązkowymi, które pozwala na częste meldowanie, ponieważ kolega jest tuż obok ciebie. Ignoruje to, że programowanie parami nie zawsze działa dla wszystkich. Prawidłowo wykonane programowanie w parach może być również bardzo męczące, więc niekoniecznie musi to być cały dzień.
Gałęzie programistów, kod jest sprawdzany i sprawdzany w głównej gałęzi dopiero po zakończeniu. Niektórzy programiści mają skłonność do pracy w tajemnicy i po tygodniu opracowują kod, który może, ale nie musi, zostać poddany recenzji z powodu fundamentalnych problemów, które mogły zostać zauważone wcześniej.
Przegląd przy każdym zameldowaniu, co gwarantuje częste przeglądy. Niektórzy programiści zapominają i polegają na bardzo częstych meldunkach, co oznacza, że inni muszą dokonywać przeglądu kodu co 15 minut.
Przejrzyj w nieokreślonym czasie po zameldowaniu. Recenzje będą wypychane dalej, gdy nastąpi termin ostateczny. Kod, który zależy od już zatwierdzonego, ale jeszcze nie sprawdzonego kodu, zostanie zatwierdzony. Recenzje oznaczą problemy, a problemy zostaną umieszczone w zaległościach, które zostaną naprawione „później”. Ok, skłamałem: To nie jest proste rozwiązanie, wcale nie jest rozwiązaniem. Przejrzyj w określonym czasie po zakończeniu odprawy, ale jest to mniej proste, ponieważ musisz zdecydować, jaki jest ten określony czas
W praktyce robisz to, czyniąc to jeszcze prostszym i bardziej złożonym jednocześnie. Ustanawiasz proste wytyczne i pozwalasz każdemu zespołowi programistów ustalić, co należy zrobić, aby postępować zgodnie z tymi wytycznymi. Przykładem takich wytycznych jest:
Możliwych jest wiele alternatywnych form takich wytycznych. Skoncentruj się na tym, czego naprawdę chcesz (kod recenzowany, obserwowalny postęp pracy, odpowiedzialność) i pozwól zespołowi dowiedzieć się, w jaki sposób może ci dać to, czego chce.
źródło
w rzeczywistości robimy hybrydę na LedgerSMB. Osoby zatwierdzające zatwierdzają zmiany, które są następnie sprawdzane. Osoby niebędące osobami przekazującymi zmiany przesyłają zmiany do osób zobowiązujących się do przeglądu przedtem. Zwykle oznacza to dwa poziomy przeglądu. Najpierw dostaniesz mentora do przeglądu i mentora. Następnie mentor zostaje poddany przeglądowi kodowi po raz drugi po tym, jak się na niego podpisał i informacje zwrotne są w obiegu. Nowi sprawcy zazwyczaj spędzają dużo czasu, najpierw sprawdzając zobowiązania innych osób.
Działa całkiem dobrze. Rzecz w tym, że późniejsza recenzja jest zwykle bardziej pobieżna niż wcześniejsza, więc upewnij się, że późniejsza recenzja jest zarezerwowana dla tych, którzy się sprawdzili. Ale jeśli masz dwupoziomową recenzję dla nowych ludzi, oznacza to, że bardziej prawdopodobne jest złapanie problemów i dyskusje.
źródło
Zobowiązać się gdzie? Jest gałąź, którą stworzyłem, żeby trochę popracować. Zobowiązuję się do tej gałęzi, kiedy tylko mam na to ochotę. To niczyja sprawa. W pewnym momencie gałąź ta zostaje zintegrowana z gałęzią programistyczną. A gdzieś pomiędzy znajduje się przegląd kodu.
Recenzent recenzuje oczywiście po tym, jak zaangażowałem się w moim oddziale. Nie siedzi przy moim biurku, więc nie może go przejrzeć, zanim zaangażuję się w moim oddziale. I dokonuje przeglądu przed połączeniem i angażuje się w dział rozwoju.
źródło
Po prostu w ogóle nie rób recenzji kodu. Albo uważasz, że twoi programiści potrafią pisać dobry kod, albo powinieneś się ich pozbyć. Błędy w logice powinny zostać wykryte przez zautomatyzowane testy. Błędy to styl, który powinien być wychwytywany przez narzędzia do analizy kłaczków i analizy statycznej.
Zaangażowanie ludzi w automatyczne procesy jest po prostu marnotrawstwem.
źródło