Jaki jest najlepszy sposób na sprawdzenie kodu, zanim zostanie on przypisany do pnia? (SVN)

Jaki jest najlepszy sposób na sprawdzenie kodu, zanim zostanie on przypisany do pnia SVN? Jednym z pomysłów, o których myślę, jest zlecenie dewelopera zatwierdzenia kodu w oddziale, a następnie przejrzenie jego kodu podczas scalania wersji gałęzi do pnia. Czy to dobra praktyka? Jeśli nie, co jeszcze można zrobić, aby przejrzeć kod, zanim zostanie on przypisany do pnia?

Istnieją jednak dwie szkoły - to, co proponujesz lub „weryfikujesz przed zatwierdzeniem”. Większość różnic można postrzegać jako negatywy i / lub pozytywne. - np. Brak śledzenia zmian spowodowanych przeglądem - czy chcesz postrzegać je jako dyskretne zatwierdzenia, czy interesuje Cię tylko końcowa praca?

Przejrzyj przed zatwierdzeniem - nie wymaga rozgałęzienia (chociaż można to zrobić w razie potrzeby), musi dać recenzentom dostęp do folderów roboczych. Kod można zmienić podczas i po sprawdzeniu bez śledzenia. Korekty spowodowane przeglądem nie pojawiają się w repozytorium.

Recenzja po zatwierdzeniu (w oddziale) - Musisz obrócić gałąź dla każdej recenzji (chociaż może to już być w przepływie pracy). Kod przesłany do przeglądu nie może zostać zmieniony bez śledzenia zmian. Ktoś musi scalić sprawdzone gałęzie i śledzić, co zostało sprawdzone, a co nie.

W dużej mierze zależy to od kultury zespołu i doświadczenia. Jaki model ufasz, jaki jest główny cel recenzji? Osobiście wolę recenzję po zatwierdzeniu, ponieważ pozwala na śledzenie zmian w wyniku recenzji. Teraz używamy Git i Gerrit, ponieważ zapewniają one dobrą równowagę między dwiema opcjami.

1. Przed uruchomieniem uruchom polecenie „svn diff”, aby wygenerować plik poprawki.
2. Wyślij plik łaty do recenzenta, który zastosuje go do czystej kopii pnia.
3. Recenzent analizuje zmiany przy użyciu wybranej przeglądarki różnic.
4. Recenzent wykonuje kompilację i uruchamia testy.
5. Jeśli wszystko pójdzie dobrze, powiedz programistom, że mogą sprawdzić swoje zmiany. W
   przypadku problemów programista wraca do kroku 1.

Jest świat idealny, a potem świat rzeczywisty.

W idealnym świecie cały kod jest testowany, więc możesz mieć pewność, że wszystko, co zostanie zarejestrowane, zadziała albo będziesz wiedział, że jest uszkodzony, ponieważ nie przejdzie jednego lub więcej testów. Ponadto każdy, kto nie jest tak doświadczony, zostanie sparowany z kimś, kto ma doświadczenie, więc przeglądanie kodu odbywa się w locie (zatwierdzasz podczas pracy).

W prawdziwym świecie rzeczy są inne. Biznes chce, aby ta zmiana była teraz na żywoi powie Ci, z idealnie prostą twarzą, że tak, otrzymasz czas na wyczyszczenie kodu i dodanie przypadków testowych później. Prawdopodobnie nie będziesz mieć czasu na sprawdzenie kodu, a odsetek kodu objętego testami stale maleje. Głównym powodem przeglądu kodu jest to, że młodsi programiści uczą się od starszych programistów (gdy jest na to czas), prosząc bardziej doświadczoną osobę o zmiany i sugerowanie „lepszych sposobów robienia rzeczy (TM)”. Będziesz miał starszych programistów popełniających nie sprawdzony kod. Rozgałęzienie tylko w celu przeglądu kodu, a następnie scalenia to ogromna strata czasu. Jednym ze sposobów rozwiązania tego problemu jest ogłoszenie regularnego cotygodniowego dwugodzinnego (lub mniej więcej) spotkania zespołu, podczas którego wybierasz jedną lub dwie ostatnie zmiany, nad którymi pracowali ludzie w krótkim czasie, i zachęca ich autorów do „przedstawienia” ich podejście polegające na wspólnym spojrzeniu na kod na projektorze lub czymś innym. Może to prowadzić do interesujących dyskusji (zwykle trochę nie na temat), ale ogólnie poprawia zrozumienie, jak zrobić to dobrze. Dodatkowo presja związana z koniecznością przedstawienia kodu sprawia, że ​​niektórzy ludzie robią to lepiej ;-)

Lub możesz mieć szczęście i pracować w środowisku rzeczywistym, w którym nie jest tak nerwowo, programiści są doceniani za to, co robią zamiast nadużywać, i jest czas, aby zrobić wszystko dobrze. W takim przypadku moja odpowiedź brzmiałaby: wypróbuj różne metody sugerowane w odpowiedziach tutaj i sprawdź, która z nich pasuje do twojego zespołu i jak najlepiej pracujesz.

Oddziały powinny działać OK, na podstawie mojego doświadczenia w stosowaniu ich w przeglądach przed zatwierdzeniem na wcześniejszej pracy.

_{Zauważ, że wtedy używaliśmy recenzji przed zatwierdzeniem tylko dla krytycznych poprawek do kodu kandydata do wydania produkcyjnego, więc nie było wielu gałęzi (rutynowe zmiany były przekazywane przez recenzje po zatwierdzeniu).}

Ponieważ wydaje się, że będziesz używał recenzji przed zatwierdzeniem wszystkich zmian, może być konieczne zarządzanie dużą liczbą oddziałów. Jeśli spodziewasz się, że programista wprowadzi średnio jedną „przeglądalną” zmianę tygodniowo, w końcu będziesz mieć około 50 oddziałów rocznie dla każdego programisty w zespole. Jeśli używasz mniejszych kawałków pracy - na przykład zajmujących 1, 2, 3 ... dni - pomnóż odpowiednio 50 przez 2, 3, 5 ... odpowiednio.

Poniżej kilka innych kwestii, które należy wziąć pod uwagę, jeśli chcesz to najlepiej .

1. rozpatrywanie spraw, w których opóźniony przegląd blokuje kod potrzebny innym członkom zespołu

Ustanawiaj, monitoruj i rozwiązuj konflikty związane z terminami przeglądu kodu. Zgodnie z moim wspomnieniem przeglądów poprzedzających zatwierdzenie rutynowych zmian, którymi zajmowałem się w jednym z poprzednich projektów, rozsądny termin wynosi około 3 dni, a czas, aby zacząć się martwić, to wtedy, gdy przegląd nie zostanie zakończony później niż 1 dzień po przesłaniu.

_{Dla porównania, w przeglądach po zatwierdzeniu wymagania te są znacznie bardziej zrelaksowane (używam terminu 2 tygodni i zaczynam się martwić po 1 tygodniu) - ale ponieważ celujesz w recenzje przed zatwierdzeniem, prawdopodobnie nie jest to interesujące.}

2. Scal konflikty podczas przesyłania sprawdzonego kodu

Co zrobić, jeśli zatwierdzenie sprawdzonego kodu jest blokowane przez sprzeczne zmiany popełnione przez kogoś innego, gdy kod czekał na sprawdzenie?

Oto kilka opcji do rozważenia

• cofnij się do początku i wymagaj od programistów ponownego wdrożenia i przeglądu zmiany.
  W takim przypadku może być konieczne zajęcie się negatywnym wpływem na morale zespołu, co może (będzie!) mieć.
• przekazać odpowiedzialność za scalenie innemu członkowi zespołu („master master”).
  W takim przypadku musisz również zdecydować, czy połączenia same w sobie powinny zostać poddane przeglądowi przed zatwierdzeniem, czy nie - a jeśli tak, to co zrobić, jeśli to połączenie z kolei spotyka inny konflikt.
• zignoruj ​​zmiany wprowadzone w sprawdzonym kodzie na etapie scalania
  W takim przypadku może być konieczne zajęcie się negatywnym wpływem na morale zespołu związane z faktem, że popełniony kod różni się od sprawdzonego.
• wymyślić sposób na uniknięcie konfliktów.
  Prostym podejściem jest umożliwienie tylko jednemu deweloperowi modyfikowania określonego zestawu plików - chociaż to nie ochroni cię przed zmianami, które nie modyfikują plików bezpośrednio, ale wpłyną na nie poprzez wewnętrzne zmiany API . Może się również okazać, że tego rodzaju „pesymistyczne blokowanie” sprawia, że ​​zmiany w całym systemie i głębokie refaktoryzowanie są dość kłopotliwe.

_{Dla porównania nie byłoby takich problemów w przeglądach po zatwierdzeniu (ponieważ dotyczą one kodu, który jest już scalony z definicji) - ale ponieważ celujesz w recenzje przed zatwierdzeniem, prawdopodobnie nie jest to interesujące.}

3. załaduj programistę, który czeka na sprawdzenie

Ustal wyraźne zasady określające, czy programista, który przesłał recenzję, powinien przejść do nowego zadania lub zrobić coś innego (np. Ścigać recenzenta).

_{Dla porównania, przeglądy po zatwierdzeniu prawie nie wymagają wyraźnych zasad (ponieważ naturalne jest przejście do następnego zadania po popełnieniu kodu i biorąc pod uwagę, że termin przeglądu wynosi tydzień lub dwa) - ale ponieważ celujesz w przeglądy przed zatwierdzeniem, prawdopodobnie jest to nieinteresujące.}

Wszelkie zmiany wymagające przeglądu musiałyby znajdować się w osobnej gałęzi. Tak więc gałąź powinna już istnieć, zanim przyjdzie czas na sprawdzenie. Następnie krok musiałby być:

1. Przejrzeć
2. Popraw (ewentualnie)
3. powróć do recenzji (ewentualnie)
4. Scal w bagażnik

Scalanie jest trudnym zadaniem. Im dłużej gałąź pozostaje niezależna, tym trudniej będzie połączyć ją z powrotem w pień. (Testowanie może być trudniejsze).

Możliwe jest połączenie krzyżowe. Przed połączeniem się z pniem (krok 4 lub nawet wcześniej, powiedz przed krokiem 3 lub krok 1), połącz pień z gałęzią. Deweloper może to zrobić i przetestować. Następnie gałąź dogania pień i łatwiej jest połączyć go z pniem. Scalanie w bagażniku najlepiej wykonać sam lub ktokolwiek jest odpowiedzialny za bagażnik.

Niektóre osoby próbują rebase zamiast scalania krzyżowego. Niektórzy twierdzą, że bazowanie jest złe. To kolejna debata.