PR Review - Usprawnienia - Łagodzenie bólu Code Review
Wspólnie z Krzysztofem Witczakiem stworzyliśmy serię artykułów, w których zastanawiamy się skąd tak duża popularność Pull Request Review, oraz jakie problemy Pull Request Review tworzy w naszych zespołach. Następnie podjęliśmy temat rozwiązywania kwestii PR Review i mierzenia, czy jest lepiej.
W serii PR Review:
Usprawnienia - Łagodzenie bólu Code Review [TEN ARTYKUŁ]
W poprzednim artykule opisaliśmy kilka alternatywnych podejść do PR Review, zmiana metodyki nie jest jednak w każdym przypadku możliwa. Dlatego w tej części opiszemy jak sprawić, by były mniej uciążliwe.
Rozwiązania na złagodzenie bólu review
Zmniejszenie skali zmian
Od lat słyszymy, że dzielenie user stories na mniejsze kawałki to coś, co warto robić. To podejście wprowadza dla nas kilka ważnych cech:
Małe zmiany oznaczają mniejsze PR’y, które łatwiej rzetelnie przejrzeć.
Dzielenie zadań na mniejsze zmusza nas do przemyślenia szczegółów, co często odkrywa “unknown unknowns” - mniejsze zadania mają mniej pułapek.
Podzielone zadania często można rozdzielić na kilku developerów i dostarczać w tym samym czasie.
Wszystkie te cechy przyspieszają delivery. Popularna metoda “no estimate” sugeruje, aby przejść z estymacji godzinowych na punkty, a potem z punktów na zadanie na około “jeden dzień”, tak aby wiadomo było, że developer wrzuca zmiany minimum raz dziennie.
Nie opisujemy tutaj konkretnych technik – Radek przygotował na ten temat inny odcinek newsletteru ✂ 5 technik podziału funkcjonalności na mniejsze.
Praktyki stopniowego wdrażania
Chcemy dzielić zadania na małe kawałki i wrzucać je bezpiecznie na produkcję, nawet bez review. Musimy więc mieć możliwość ukrywania niedokończonego kodu przed użytkownikiem końcowym.
To wymaga od nas zmiany mindsetu – wdrożenie kodu nie będzie tożsame z uruchomieniem. Dobrze to opisuje artykuł Deployment vs. Release:
W wprowadzeniu tej idei w życie wspierają nas praktyki stopniowego wdrażania:
Feature flags (albo feature toggle) - pozwalają modyfikować konfigurację aplikacji na produkcji bez deploymentu oraz uruchamianie innych ścieżek kodu. Dzięki takim zmiennym możemy określić, że feature in progress widzi np. tylko zespół developerski i konto testowe.
Dark Launching – wywołujemy nową funkcję równolegle do poprzedniej. Ma to na celu ocenę poprawności działania i oceny wpływu dodatkowego obciążenia i wydajności na system.
Keystone Interface – funkcja biznesowa jest udostępniona produkcyjnie, ale nie jest widoczna dla użytkowników. Działa to zarówno na zmiany backendowe (zmiany w API), ale także frontowe (ukrywamy części aplikacji).
Istnieją liczne usługi (płatne i open source) pomagające implementować feature flagi takie jak LaunchDarkly czy Unleash.
Pamiętaj, że ten wzorzec ma pomagać w efektywnym wdrażaniu - flagi wdrożeniowe powinny mieć krótki okres życia - w przeciwnym wypadku nasz kod szybko stanie się bardzo zagmatwany.
Shift left decyzji technicznych
Ostatnio wspominaliśmy, że zespoły często mają problem z planowaniem, i “nadrabiają” na review. Wielu problemów jesteśmy w stanie uniknąć przenosząc kluczowe decyzje wcześniej w procesie.
Istnieje definicja “feedbacku 80/20”. Zasada ta mówi, że powinniśmy dopasować nasz feedback do etapu prac.
Jeżeli prace są na 20% progresu, to mamy jeszcze daleką drogę przed sobą i możemy dawać feedback, który wiele zmienia.
Jeżeli prace są na 80% progressu, taki feedback tylko irytuje. Gdzie byłeś, gdy definiowaliśmy wymagania? Teraz jest już za późno…
Na 80% progresu oczekujemy feedbacku niewielkiego, dopracowującego kierunek, zamiast sugerującego, aby go zmienić.
O tym również wspomina często Adam Dymitruk, twórca techniki Event Modelingu – tu jeden z jego postów:
Nowoczesne metody Product Discovery starają się angażować developerów bardzo wcześnie na etapie odkrywania wymagań. Proces, który możemy polecić, wygląda następująco:
Jeden z developerów staje się „skautem”, odpowiedzialnym za głębsze rozpoznanie danego zadania. Przeprowadza analizę pod kątem docelowego kształtu rozwiązania.
„Skaut” tworzy „ad-hoc design” – krótki artefakt opisujący wykonywane zmiany. Może to być:
Fragment z jednej lekkich technik np. Event Storming Process Level, model C4, diagram sekwencji.
Podział prac – programistyczne, infrastrukturalne, testy, metryki.
Zbiór zadań zespołów zależnych.
Dzieli się tym z innymi developerami. Potwierdzane są założenia i akceptowany sposób wykonania.
Następnie zadanie jest wykonywane wg. schematu, na który umówił się zespół.
Jeśli podczas implementacji wychodzą jakieś duże niespójności to eskalujemy to do zespołu.
Nie chcemy podejmować istotnych decyzji architektonicznych bez potwierdzenia ze strony zespołu.Cała idea “shift left” polega właśnie na wyeliminowaniu ad-hocowych decyzji podczas implementacji. Dzięki temu unikamy niezgody na późnych etapach dostarczania.
Krótko żyjące branche
Aby ułatwić sobie sprawdzanie kodu przed zmergowaniem trzeba mieć mniej kodu do zmergowania #ThankYouCaptainObvious 😅
Aby to wdrożyć w życie musimy przyjąć podejście, które skupia nas na tym by gałęzie kodu były możliwie jak najkrócej żyjące. I jest na to rozwiązanie:
Trunk-based development (TBD) to alternatywna strategia branchowania do GitFlow czy GitHub Flow.
Strategia ta istnieje w kilku wariantach w zależności od rozmiaru zespołu, ale główna idea polega na tym, że branch na którym pracujemy powinien być “short-lived”. Najczęściej punktem krytycznym tutaj są dwa dni, po których spodziewamy się, że developer zacommituje swoje zmiany do main brancha (trunka). Jak to wpływa na PR review?
TBD zakłada, że jeżeli robimy PR review, to musi on być szybki – zasada ta nazywana jest Continuous Code Review.
Zespół musi traktować review jako priorytet i wykonać go jak najszybciej, idealnie w ciągu kilku minut.
Dopuszcza się kilkadziesiąt minut, ale jeżeli wchodzimy w godziny, to zaczynamy łamać tę zasadę. Wszystko po to, aby nie tracić rozpędu i prędkości.
Domyślasz się zapewne, że takie zespoły często w ogóle nie przeprowadzają review, a stosują inne praktyki wymienione w tym artykule.
O czym również warto pamiętać:
Raporty DORA od lat wymieniają TBD jako cechę najszybszych i najlepszych jakościowo zespołów;
Zwolennicy prawdziwego continous integration (patrz kolejne części😉) wymieniają TBD jako niezbędny pierwszy krok w kierunku codziennego wdrażania;
Jak utrzymywać jakość usprawniająć PR Review?
Argumentem strony zwolenników PR Review jest to, że taki model sprawdzania kodu zapewnia jakość. Dlatego w kolejnej części serii opisaliśmy przydatne praktyki, które pozwolą wam zachować jakość pomimo braku PR Review: