Kiedy robić recenzje kodu podczas ciągłej integracji?


33

Próbujemy przejść na środowisko ciągłej integracji, ale nie jesteśmy pewni, kiedy przeprowadzać recenzje kodu. Z tego, co przeczytałem o ciągłej integracji, powinniśmy próbować sprawdzać kod tak często, jak kilka razy dziennie. Zakładam, że oznacza to nawet funkcje, które nie są jeszcze kompletne.

Pytanie brzmi: kiedy przeprowadzamy recenzje kodu?

Nie możemy tego zrobić przed wpisaniem kodu, ponieważ spowolniłoby to proces, w którym nie będziemy mogli wykonywać codziennych kontroli, nie mówiąc już o wielu kontrolach dziennie.

Ponadto, jeśli kod, który sprawdzamy, po prostu się kompiluje, ale nie jest kompletny, wykonanie przeglądu kodu jest wówczas bezcelowe, ponieważ większość recenzji kodu najlepiej wykonać po sfinalizowaniu funkcji. Czy to oznacza, że ​​powinniśmy robić recenzje kodu po zakończeniu funkcji, ale ten nie sprawdzony kod dostanie się do repozytorium?


Jeśli chodzi o meldowanie / wypychanie, w większości miejsc obowiązuje jedna główna zasada: nie przerywaj kompilacji! Nie rejestruję czegoś, co nie da się zbudować. Poza tym w większości miejsc chciałem mieć małe i ograniczone zameldowanie, ale nigdy nie mówiłem nic o tej kwocie.
Jakiś programista koleś

ale kiedy nastąpi weryfikacja kodu, zanim się zameldujesz lub kiedy skończysz z tą funkcją? Czy to oznacza, że ​​masz kod, który nie został sprawdzony, i że rozwiązałeś problemy wykryte po sprawdzeniu?

Różni się, ale większość miejsc chce dokonać przeglądu kodu w prywatnych oddziałach, zanim zostanie połączona z jedną z głównych gałęzi,
Pewien programista koleś

Odpowiedzi:


12

IMO, powinieneś przejrzeć kod, zanim zostanie opublikowany w mainline, tak aby mainline miał tylko kod najwyższej jakości.

OTOH, można powiedzieć, że „po co zawracać sobie głowę sprawdzaniem, czy automatyzacja testów CI nie działała na nim?”, Więc być może najlepiej byłoby dać programistom każdą gałąź prywatną, którą zbuduje serwer CI i przetestować dla nich. . W ten sposób najpierw zatwierdzają i pchają tam, a potem, gdy mija, sprawdzają, a następnie łączą się z linią główną (gdzie otrzyma kolejny przebieg przez serwer CI).

Zdecydowanie powinieneś przejrzeć niekompletny kod, aby upewnić się, że rusztowanie dla przyszłych funkcji jest na miejscu, a przynajmniej, że nic nie zostanie wprowadzone, co uniemożliwiłoby wdrożenie wspomnianych przyszłych funkcji.

Zauważ też, że recenzje kodu nie muszą być powolne ani synchroniczne - narzędzie takie jak gerrit lub tablica kontrolna itp. Może sprawić, że będą asynchroniczne i dość bezbolesne.

(Pełne ujawnienie: Pracowałem dla SmartBear, twórcy Code Collaborator, narzędzia do sprawdzania kodu)


4
Codereview przez e-mail to zła praktyka (choć lepsza niż nic, co prawda), ponieważ trudno jest powiedzieć, kiedy recenzja jest „skończona”. Zdobądź prawdziwe narzędzie do sprawdzania kodu, takie jak gerrit lub tablica recenzji, i używaj go i przestań
wysyłać

1
Mimo to nie sądzę, że jest to idealny proces, niezależnie od DVCS, czy nie. Jedną z konieczności przeglądu kodu jest nie tylko spojrzenie na kod, ale także jego uruchomienie lub automatyczne przetestowanie i sprawdzenie, co się stanie. Nie możesz tego zrobić za pomocą serii różnic.
Jordan

2
+1 za sugestię, że przeglądy powinny być przeprowadzane po uruchomieniu testów automatycznych.
William Payne

1
Jordania: prawdziwe narzędzia do przeglądania kodu (gerrit itp.) Zapewniają więcej niż tylko różnice - pozwalają przeczytać cały kontekst, dzięki czemu można dowiedzieć się, na co wpływa zmiana kodu. Jeśli to konieczne, możesz, tak, pobrać łatkę i ją skompilować, ale ponieważ i tak wszystko przechodzi przez CI, zakłada się, że błędy mogą zostać wykryte przez automatyzację, więc bardziej koncentruje się na łatwości konserwacji i krawędzi przypadków niż automatyzacji lub codzienne testy mogą się nie złapać.
pjz

1
Czy jednym z punktów CI nie jest synchronizacja wcześnie i często z linią główną? Twoje podejście opóźniłoby synchronizację, co ma wiele wad.
Jacob R

11

Skonfigurować programowanie par?

Cały kod jest sprawdzany podczas pisania bez rozszerzania procesu lub wprowadzania kolejnego kroku.


7

Oto fragment autora o ciągłej dostawie:

Jez Humble Pisze jako:

Obecnie piszę post na blogu na ten temat. Krótka odpowiedź brzmi:

  • Najlepszym sposobem przeglądania kodu jest programowanie parami
  • To zły pomysł, aby przejść do łączenia z linią główną - na przykład poprzez utworzenie osobnego oddziału - w ramach formalnego procesu przeglądu. Utrudnia to ciągłą integrację (najlepszy sposób zmniejszenia ryzyka złych zmian, do czego tak naprawdę dążysz).
  • Myślę, że Gerrit to fajne narzędzie, ale powinno się go używać po odprawie (tak właśnie zostało zaprojektowane). Częścią starszego programisty jest przegląd wszystkich meldowań. Mogą na przykład zasubskrybować kanał.

Podsumowując: przegląd kodu jest dobry. Tak dobrze, powinniśmy robić to w sposób ciągły, poprzez programowanie par i sprawdzanie zatwierdzeń. Jeśli starszy deweloper znajdzie złe zatwierdzenie, powinna sparować z osobą, która go popełniła, aby pomóc im rozwiązać problem.

Wstawianie scalania do linii głównej podczas formalnej oceny jest złe, a tworzenie gałęzi do tego celu jest wyjątkowo złe, z tego samego powodu, że gałęzie cech są złe.

Dzięki,

Jez.

oryginalny link to: https://groups.google.com/forum/#!msg/continuousdelivery/LIJ1nva9Oas/y3sAaMtibGAJ


5

Nie wiem, czy to najlepszy sposób, aby to zrobić ... ale wyjaśnię, jak to robimy. Jeden lub więcej programistów pracuje nad danym oddziałem i zatwierdza swój kod tak często, jak to możliwe, aby uniknąć marnowania czasu na scalanie, które inaczej by się nie wydarzyło. Dopiero gdy kod jest gotowy, jest on zapisywany w pamięci. Teraz to dotyczy zmian i odgałęzień.

Jeśli chodzi o przegląd kodu, używamy Sonaru jako naszego narzędzia do ciągłej integracji (i Maven / Jenkins do interakcji z Sonarem) do dostarczania nam świeżych wyników testów, pokrycia kodu i automatycznej oceny kodu każdego ranka (kompilacje są wykonywane co noc), dzięki czemu programiści mogą spędzić maksymalnie godzinę każdego ranka, aby naprawić swoje problemy / zapachy kodu. Każdy programista bierze odpowiedzialność (także z dumy!) Za napisaną przez siebie funkcję. Teraz jest to automatyczna weryfikacja kodu, która świetnie sprawdza się w wykrywaniu potencjalnych problemów technicznych / architektonicznych, ale ważniejsze jest sprawdzenie, czy te nowe zaimplementowane funkcje działają prawidłowo.

Do tego są dwie rzeczy: testy integracyjne i przegląd kodu równorzędnego. Testy integracyjne pomagają mieć wystarczającą pewność, że nowy kod nie psuje istniejącego kodu. Jeśli chodzi o przegląd kodu równorzędnego, robimy to w piątkowe popołudnia, co jest nieco bardziej zrelaksowanym czasem :-) Każdy programista jest przypisany do gałęzi, nad którą nie pracuje, zajmuje trochę czasu, aby przeczytać wymagania najpierw nowa funkcja, a następnie sprawdza, co zostało zrobione. Jego najważniejszym zadaniem jest upewnienie się, że nowy kod działa zgodnie z oczekiwaniami, biorąc pod uwagę wymagania, nie łamie własnych „reguł” (używaj tego obiektu do tego, a nie tego), jest łatwy do odczytania i pozwala na łatwe przedłużanie.

Mamy więc dwie recenzje kodu, jedną automatyczną i jedną „ludzką” i staramy się unikać popełniania nieprzeczytanego kodu w gałęzi HEAD. Teraz ... Zdarza się to czasami z różnych powodów, jesteśmy daleki od ideału, ale staramy się zachować równowagę między jakością a kosztami (czas!)

@pjz również stanowi dobrą odpowiedź i wspomina o narzędziach do sprawdzania kodu. Nigdy nie korzystałem z nich, więc nie mogę nic o tym powiedzieć ... chociaż w przeszłości kusiło mnie, aby pracować z Crucible, ponieważ już używamy JIRA .


Ciekawy pomysł, że recenzje powinny być zaplanowane na określoną porę dnia / dnia ...
William Payne

@WilliamPayne dziękuję. Kolejną rzeczą, która działa dla nas, jest planowanie spotkań związanych z przeglądem kodu, dzięki czemu w kalendarzu wyraźnie widać, że jesteśmy „zajęci”. Pomaga ostrzegać ludzi, że chociaż jesteśmy tutaj ... w rzeczywistości nie jesteśmy :-)
Jalayn

4

Myślę, że główna koncepcja, która pomoże, to koncepcja „inscenizacji”.

Tak, nie chcesz sprawdzać uszkodzonego kodu. Ale powinieneś także często sprawdzać kod. Czy to oznacza doskonałość? ;) Nie. Wystarczy użyć wielu obszarów i DVCS, takich jak git.
W ten sposób wprowadzasz zmiany (lokalnie) i często je zatwierdzasz podczas testowania i rozwijania aż do zaliczenia testów. Następnie przesuwasz się do obszaru pomostowego w celu przejrzenia kodu.

Następnie powinieneś przechodzić od etapu przejściowego do innych działań związanych z kontrolą jakości, takich jak testy przeglądarki i testy użytkownika. Wreszcie możesz przejść do obszaru testowania objętościowego, a następnie produkcji.

Istnieją również przepływy pracy, takie jak wszyscy pracujący w głównej gałęzi lub używający poszczególnych gałęzi do wszystkich wysiłków.

Sama ciągła integracja może się także odbywać na wielu poziomach. Może być lokalny dla maszyny deweloperskiej „do momentu przejścia testów”, a także może znajdować się w obszarach pomostowych i qa, gdy kod do nich trafi.



2

Używamy git flow do naszych repozytoriów i dokonujemy przeglądów kodu, jeśli chodzi o połączenie z gałęzią programistyczną.

Wszystko w fazie rozwoju jest kompletne, możliwe do wdrożenia i sprawdzone pod kątem kodu.

Mamy również skonfigurowane CI dla naszych gałęzi rozwoju i master.


2

Naprawdę myślę, że naprawdę potrzebujesz DVCS (np. Mercurial, git), aby zrobić to naturalnie. Z CVCS potrzebujesz gałęzi i masz nadzieję, że dla każdego boga, którego masz, nie będzie piekła.

Jeśli korzystasz z DVCS, możesz warstwowo zintegrować proces, aby kod już sprawdzał go, zanim dotrze do serwera CI. Jeśli nie masz DVCS, kod dostanie się na serwer CI przed sprawdzeniem, chyba że recenzenci kodu sprawdzą kod na komputerze każdego programisty przed przesłaniem zmian.

Pierwszym sposobem na to, szczególnie jeśli nie masz oprogramowania do zarządzania repozytoriami, które mogłoby pomóc w publikowaniu osobistych repozytoriów (np. Bitbucket, github, rhodecode), jest posiadanie hierarchicznych ról integracyjnych. Na poniższych schematach możesz poprowadzić poruczników do przeglądu pracy programistów, a dyktator jako główny integrator sprawdzi, jak porucznicy połączyli pracę.

wprowadź opis zdjęcia tutaj

Innym sposobem na to, jeśli masz oprogramowanie do zarządzania repozytoriami, jest użycie przepływu pracy takiego jak poniżej:

wprowadź opis zdjęcia tutaj

Oprogramowanie do zarządzania repozytoriami zazwyczaj pomaga wysyłać powiadomienia, gdy w repozytoriach występuje aktywność (np. E-mail, rss), a także umożliwia wysyłanie żądań . Przegląd kodu może odbywać się organicznie podczas żądań ściągania, ponieważ żądania ściągania zwykle powodują, że ludzie angażują się w rozmowy, aby zintegrować kod. Weźmy tę publiczną prośbę jako przykład. Menedżer integracja rzeczywiście nie może pozwolić kod dotrzeć do błogosławionego repozytorium (aka „centralnym repozytorium”), jeśli potrzeby kodów, które należy poprawić.

Co najważniejsze, dzięki DVCS nadal możesz obsługiwać scentralizowany przepływ pracy, nie musisz mieć innego fantazyjnego przepływu pracy, jeśli nie chcesz ... ale dzięki DVCS możesz oddzielić centralne repozytorium programistyczne od CI serwer i dać komuś uprawnienia do przekazywania zmian z repozytorium deweloperów do repozytorium CI po zakończeniu sesji przeglądu kodu .

PS: Kredyty za zdjęcia trafiają na git-scm.com


1
Ludzie w github używają zapytań ściągających, aby dokonać przeglądu kodu i wydaje się, że działa dobrze, zdaniem Scotta Chakona , Zach Holmana i innych.
Spoike

1

Dlaczego nie mieć więcej niż jednego repozytorium? Jeden do „codziennej” pracy, prowadzenia ciągłego serwera integracyjnego, uruchamiania wszystkich testów jednostkowych i testów integracyjnych w celu uzyskania ładnej ciasnej pętli sprzężenia zwrotnego, a drugi do „stabilnej” pracy, w której zatwierdzenia są rzadsze, ale muszą przejść przegląd.

W zależności od ścieżki, którą podążają zmiany podczas poruszania się po systemie, może to być trochę skomplikowane rozwiązanie i może działać najlepiej, gdy używa się narzędzi takich jak Git lub Mercurial Queues, (zastrzeżenie: nie użyłem ani w gniewie) ale wiele organizacji robi coś podobnego.


1

Czy to oznacza, że ​​powinniśmy robić recenzje kodu po zakończeniu funkcji, ale ten nie sprawdzony kod dostanie się do repozytorium?

Znacznie powyżej jest sposób, w jaki widziałem to w co najmniej trzech projektach, które intensywnie korzystały z ciągłej integracji i według moich wspomnień działało to jak urok. Ta praktyka jest znana jako recenzje kodu po zatwierdzeniu - wyszukaj ten termin, jeśli interesują Cię szczegóły.

  • Z drugiej strony, jedyny przypadek, kiedy byłem w projekcie, próbując „poślubić” recenzje kodu pre-commit z CI, okazało się dość bolesne. Cóż, gdy wszystko poszło w 100% sprawnie, było OK - ale nawet rzadkie przerwy (na przykład, gdy recenzenci główni i zapasowi byli niedostępni przez kilka godzin) zauważali stres. Zauważyłem też, że morale drużyny nieco ucierpiało - konfliktów było trochę za dużo.

-2

Po pierwsze, powinniśmy wyjaśnić pojęcie „ciągłej integracji”. W tradycyjnych metodach opracowywania ciągła integracja oznacza, że ​​możemy codziennie integrować i budować nasze repozytorium kodu źródłowego, co pozwoli uniknąć pułapek związanych z „piekłem integracji”. Przeglądy kodu są zawsze między okresem kodowania a testowaniem jednostek. Musimy zagwarantować, że kod scalający się z oddziałem będzie mógł zostać skompilowany bez błędów. Rzadko zdarza się, że części funkcji łączą się z gałęzią, ponieważ trudno jest zachować spójność interfejsu i błędy kompilacji.

Ciągła integracja jest popularna w procesie ekstremalnego programowania. Programowanie oparte na testach dodaje programowanie w parach, które jest faktyczną częścią procesu przeglądu kodu, dzięki czemu ciągła integracja jest łatwa do wdrożenia. Sam Extreme Programming to ciągły proces przeglądu i integracji kodu. Przegląd kodu istnieje wszędzie.

W niektórych społecznościach typu open source przeglądy kodu są wykonywane tuż przed scaleniem kodu z oddziałem. Zawsze najbardziej doświadczeni ludzie w tym zespole przeprowadzają przeglądy kodu i decydują, czy kod może zostać scalony z główną gałęzią. W ten sposób okres ciągłej integracji jest nieco dłuższy, ale jakość kodu jest nieco lepsza.

Wróć do pytania. Nie ma standardowej odpowiedzi na pytanie, kiedy należy dokonać przeglądu kodu, i zależy to od oryginalnego procesu programowania oraz faktycznej implementacji ciągłej integracji.

Korzystając z naszej strony potwierdzasz, że przeczytałeś(-aś) i rozumiesz nasze zasady używania plików cookie i zasady ochrony prywatności.
Licensed under cc by-sa 3.0 with attribution required.