Przejrzyj przed lub po zatwierdzeniu kodu, co jest lepsze?

71

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,

  1. Mamy doświadczonych programistów, a także nowych pracowników z niemal zerowym doświadczeniem w programowaniu.
  2. Chcielibyśmy wykonać szybkie i krótkie iteracje, aby wypuścić nasz produkt.
  3. Wszyscy członkowie zespołu znajdują się w tej samej witrynie.

Zalety recenzji kodu przed zatwierdzeniem nauczyłem się:

  1. Mentor nowych pracowników
  2. Staraj się zapobiegać błędom, awariom, złym projektom na wczesnym etapie cyklu programowania
  3. Ucz się od innych
  4. Kopia zapasowa wiedzy, jeśli ktoś zrezygnuje

Ale miałem też złe doświadczenia:

  1. Niska wydajność, niektóre zmiany mogą być przeglądane w ciągu kilku dni
  2. Trudno zrównoważyć szybkość i jakość, szczególnie dla początkujących
  3. 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:

  1. Używamy Perforce do VCS
  2. Kodujemy i zatwierdzamy w tych samych gałęziach (gałęzie naprawiające pień lub błędy)
  3. 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.
piąta
źródło
13
Czy zobowiązują się do własnego oddziału? To może być argument twoich kolegów za przeglądem po zatwierdzeniu. Osobiście powiedziałbym wstępne zatwierdzenie dla bardzo niedoświadczonych programistów.
Simon Whitehead,
przejrzyj
1
Co powiesz na oba? Dopóki są one wyraźnie zidentyfikowane, nie powinno stanowić problemu, np. Oddział przed przeglądem, scalanie po. Zapewnia natychmiastowy dostęp do innych programistów, którzy mogą potrzebować zobaczyć, co będzie dalej. Utrwala zmiany wynikające z recenzji, stanowiąc wygodną pomoc dla mentorów. Wiele recenzji można zarejestrować osobno, np. Pod kątem funkcjonalności, bezpieczeństwa i zgodności z prawem.
HABO,

Odpowiedzi:

62

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.

Thomas Owens
źródło
2
Jeśli programiści mają swoje własne gałęzie, a Ty masz odpowiednie narzędzie do sprawdzania kodu, możesz zachować kontrolę. Recenzenci muszą zarejestrować w narzędziu, czy dokonali przeglądu.
MarkJ
1
Należy dodać, że posiadanie weryfikowalnego zatwierdzenia oznacza, że ​​sam koder ma znacznie jaśniejszy umysł, wymuszając fakt, że każdy problem musi być rozwiązany osobno, małymi, udanymi krokami. Zacieśnia pętle zwrotne i wydaje się koniecznością dla każdego zwinnego zespołu.
vaab
@Thomas, Perforce to nasze obecne narzędzie VCS, wszyscy kodujemy i dokonujemy zmian w tych samych gałęziach, np. Wszystkie w gałęzi trunk lub release. Zrozumiałem, co powiedziałeś, jeśli korzystamy z Git, zgodziłbym się z twoim pomysłem, że zasady sprawdzania zależą od strategii rozgałęziania.
piąty
4
+1, działa to jeszcze lepiej, gdy każdy programista nie ma własnej gałęzi, ale zamiast tego używasz gałęzi funkcji. Naprawiamy błędy bezpośrednio do linii głównej, ponieważ są one zwykle małe, ale funkcje przechodzą do własnego oddziału, otrzymują wiele zmian, a następnie można je przejrzeć przed połączeniem z linią główną.
Izkata,
1
@ThomasOwens: Perforce obsługuje rozgałęzienia, ale nie za pomocą SVN, GIT lub Mercurial.
kevin cline
35

Jest to mantra, że nikt nie wydaje się być jeszcze cytowany: Przyjazd wcześnie i często :

Programiści, którzy pracują przez długi czas - a przez długi czas mam na myśli więcej niż jeden dzień - bez sprawdzania kontroli źródła, przygotowują się na poważne problemy z integracją. Damon Poole zgadza się :

Programiści często odkładają odprawę. Odkładają to, ponieważ nie chcą zbyt wcześnie wpływać na innych ludzi i nie chcą być obwiniani za złamanie kompilacji. Prowadzi to jednak do innych problemów, takich jak utrata pracy lub niemożność powrotu do poprzednich wersji.

Moją ogólną zasadą jest „odprawa wcześnie i często”, ale z zastrzeżeniem, że masz dostęp do prywatnej wersji. Jeśli odprawa jest natychmiast widoczna dla innych użytkowników, istnieje ryzyko wprowadzenia niedojrzałych zmian i / lub przerwania kompilacji.

Wolałbym okresowo sprawdzać małe fragmenty niż spędzać długie okresy bez żadnego pojęcia, co piszą moi współpracownicy. Moim zdaniem , jeśli kod nie jest wpisany do kontroli źródła, nie istnieje . Przypuszczam, że jest to kolejna forma Don't Go Dark ; kod jest niewidoczny, dopóki nie znajdzie się w repozytorium w jakiejś formie.

... Jeśli nauczysz się meldować wcześnie i często meldujesz, będziesz miał wystarczająco dużo czasu na opinie, integrację i recenzje po drodze ...

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ć :

Najbardziej bezpośrednim sposobem na ulepszenie jako programisty jest absolutna nieustraszoność, jeśli chodzi o zmianę kodu. Programiści, którzy boją się złamanego kodu, to programiści, którzy nigdy nie dojrzeją do roli profesjonalistów.

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.

lorddev
źródło
1
Podoba mi się ta odpowiedź - myślę, że całkiem dobrze wypełnia pozostałe tematy wymienione w zleceniu.
Joris Timmermans,
dość przekonujące wyjaśnienie, dlaczego ważne jest, aby unikać blokowania zatwierdzeń przez VCS
gnat
1
To jest dużo lepsze. Zaczyna sprawiać, że rozwój wydaje się być przedsięwzięciem, które ceni komunikację w zespole, a nie mechanistyczny system unikania winy.
Ian
19

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.

Joachim Sauer
źródło
5
To sugeruje, że drobne problemy lub sugestie są albo „naprawione” przez recenzenta, albo wcale? Spodziewałbym się, że JAKIEKOLWIEK komentarze z recenzji zostaną przekazane autorowi w celu odesłania (lub odrzucenia)
Andrew
8

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.

guillaume31
źródło
czy pochłonęło to czas na recenzje dialogów na żywo? Czy Twój zespół sprawdził cały kod?
piąty
Nie sprawdzamy całego kodu, ale prawie wszystko, co najmniej umiarkowanie skomplikowane.
guillaume31,
3
Zależy to całkowicie od tego, czego używasz do SCM. Dzięki git, tworzenie nowego oddziału, zaangażowanie się w niego i wypychanie tych zmian jest bardzo naturalnym sposobem na przegląd kodu.
kubi
8

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.

Michael Brown
źródło
3
Uwielbiam kodowanie parami, ale Mike, starszy i młodszy, nie koduje parami, to mentoring. Zdecydowanie sugeruję mentoring, ale te dwie rzeczy powinny być rozróżnione, ponieważ powody / przeciw, a wyniki są całkowicie różne między mentoringiem a programowaniem par. Zobacz czwarty post na: c2.com/cgi/wiki?PairProgrammingDoubts także c2.com/cgi/wiki?PairProgrammingIsDoneByPeers
Jimmy Hoffa
Nie zawsze. Osoba młodsza może mieć wkład. Lub zauważ „głupie błędy”.
Jeanne Boyarsky,
@JeanneBoyarsky Nie mówiłem, żeby tego nie robić, tylko, że dynamika jest inna, a wyniki są różne (nie kod, mam na myśli wynikające z tego korzyści dla całego procesu). Również jeśli „młodsza” osoba ma taką samą ilość korzystnego wkładu projektowego lub nieproporcjonalnie więcej, gdy jest sparowana z kimś, kto jest starszy, byłbym przekonany, że „młodszy” nie jest tak młodszy lub „starszy” nie jest tak starszy.
Jimmy Hoffa,
Masz rację ... ale myślę, że to najskuteczniejszy sposób dzielenia się wiedzą.
Michael Brown,
@MikeBrown - choć zgadzam się z twoimi argumentami tutaj, to linkowane wiki jest jedną z najgorszych rzeczy, jakie kiedykolwiek czytałem o programowaniu par. Wszystkie obiekcje i obawy zostały rozwiane ręcznie, ci, którzy mają co do tego wątpliwości, w zasadzie nazywali je opóźnieniami społecznymi, a kierownictwo obraziło to, że nie chcieli zastosować radykalnie nowej metodologii w swoim procesie, bez żadnych dowodów empirycznych, że faktycznie przynosi to korzyści biznesowe. Jest na równi z komentarzami na YouTube pod względem toksyczności. Nie mam pojęcia, jak ktokolwiek uważa, że ​​jest to dobra rzecz do programowania w parach, i mówię to jako ktoś, kto to lubi.
Davor Ždralo
7

Zrób jedno i drugie :

  • pre commit - rób tego rodzaju recenzje, gdy jest to coś bardzo ważnego, na przykład fragment kodu wielokrotnego użytku lub ważna decyzja projektowa
  • post commit - zrób tego rodzaju recenzje, jeśli chcesz uzyskać opinię na temat kodu, który może zostać ulepszony
BЈовић
źródło
5

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.

Andrzej
źródło
3

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.

ptyx
źródło
2

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.

superM
źródło
2

Recenzje korzystają zarówno z wcześniejszych, jak i późniejszych zobowiązań.

Zatwierdź przed przeglądem

  • Daje recenzentom pewność, że recenzują najnowszą wersję autora.
  • Pomaga upewnić się, że wszyscy recenzują ten sam kod.
  • Daje odniesienie do porównania po zakończeniu zmian dokonanych na podstawie elementów recenzji.

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

  • Daje moderatorowi i innym recenzentom punkt danych do porównania z zatwierdzeniem przed przeglądem.
  • Udostępnia wskaźniki pozwalające ocenić zarówno wartość, jak i powodzenie przeglądu przy usuwaniu defektów i ulepszaniu kodu.
DeveloperDon
źródło
1

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).

Telastyn
źródło
1

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:

  • Różnice pod względem wydajności i jakości są w wielu przypadkach nieistotne. Jeśli tak nie jest, przegląd po zatwierdzeniu ma pewne zalety pod względem jakości (inni programiści działają w pewien sposób jako „beta-testerzy”). Jeśli chodzi o efektywność, zatwierdzanie po zatwierdzeniu ma pewne zalety w małych zespołach, a wstępne zatwierdzanie ma pewne zalety w dużych lub niewykwalifikowanych zespołach.
  • Przegląd poprzedzający zatwierdzenie może prowadzić do dłuższych czasów cyklu, gdy występuje sytuacja, w której przegląd opóźnionych zadań zależnych jest opóźniony.

Na tej podstawie wyprowadziliśmy następujące reguły heurystyczne:

  • Kiedy masz już sprawdzony proces sprawdzania kodu, nie przejmuj się zmianami, prawdopodobnie nie warto tego robić
    • chyba że masz problemy z czasem cyklu => Przełącz na post
    • Lub problemy z poślizgnięciem zbyt często zakłócają pracę programistów => Przełącz na Pre
  • Jeśli jeszcze nie robisz recenzji
    • Użyj wstępnego zatwierdzenia, jeśli jedna z tych korzyści dotyczy Ciebie
      • Przegląd przed zatwierdzeniem pozwala osobom z zewnątrz bez uprawnień zatwierdzania przyczyniać się do projektów typu open source
      • Jeśli oparty na narzędziach, przegląd przed zatwierdzeniem wymusza pewną dyscyplinę przeglądu w zespołach, w przeciwnym razie luźne przestrzeganie procesu
      • Przegląd przed zatwierdzeniem z łatwością powstrzymuje dostarczenie niezbadanych zmian, co jest dobre dla ciągłego wdrażania / bardzo krótkich cykli wydania
    • Użyj pre zatwierdzenia, jeśli twój zespół jest duży i możesz żyć z problemami lub je obchodzić w czasie cyklu
    • W przeciwnym razie (np. Mały, wykwalifikowany zespół przemysłowy) zastosuj po zatwierdzeniu
  • Poszukaj kombinacji, które dadzą ci korzyści z obu światów (nie studiowaliśmy ich formalnie)

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

Tobias B.
źródło
Czy ten model został zweryfikowany na podstawie danych rzeczywistych?
Peter,
1
Model został zweryfikowany z danymi ze świata rzeczywistego w pewnym (bardzo ograniczonym) stopniu. Główny problem polega na tym, że w przypadku dużej części czynników wejściowych nie mieliśmy wartości, które zostały zmierzone w projekcie z prawdziwego świata. Głównej weryfikacji dokonano, prezentując model kilku praktykom. Dwie z nich (jedna z tłem w wersji wstępnej i jedna do publikacji) zrecenzowały ją bardziej szczegółowo. Gdybyśmy mieli lepsze dostępne dane ilościowe, prawdopodobnie nie zbudowalibyśmy wcale modelu, ale po prostu przeanalizowali dane.
Tobias B.
Dzięki, że stawia to odpowiedź w perspektywie, a tym samym czyni ją bardziej wartościową. +1
Peter
0

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:

  • Deweloperzy czerpią korzyści z kontroli wersji (tworzenie kopii zapasowych zmian, przywracanie z historii, zmiany różnic) bez zmartwień o uszkodzenie systemu dla wszystkich innych.
  • Zmiany, które powodują wady lub nie kończą się na czas, mogą zostać wycofane, ponownie ustalone priorytety lub odłożone w razie potrzeby.

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”.

RMorrisey
źródło
Tygiel jest fantastyczny i na początek kosztuje tylko 10 USD. (Chociaż wersja za 10 USD będzie zarządzać tylko 5 repozytoriami, co oznacza, że ​​możesz ją szybko przerosnąć, a następny krok jest znacznie droższy. Coś jak 1 000 USD IIRC.)
Mark E. Haase
0

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 --cachedaby 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.

Mark E. Haase
źródło
0

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.

Eric Smalling
źródło
0

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:

  • Praca jest podzielona na zadania, które powinny zająć mniej niż jeden dzień.
  • Zadanie nie jest ukończone, jeśli kod (jeśli istnieje) nie został wpisany.
  • Zadanie nie jest ukończone, jeśli kod (jeśli istnieje) nie został sprawdzony.

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.

Piotr
źródło
-1

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.

Chris Travers
źródło
-1

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.

gnasher729
źródło
-3

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.

Mike Roberts
źródło
2
Niestety, pytanie dotyczyło tego, czy zrobić recenzje przed czy po - nie, czy to zrobić, czy nie. Jeśli masz opinię na temat przed / po, dodaj to.
Marco