Przeczytaj wersję webową.

PR Review – mierzenie i rozwiązania

2024-07-13

Cześć!

Dziś mam dla Ciebie kolejne wydanie współpisane z Krzyśkiem Witczakiem. Kontynuujemy temat PR Review i skupiamy się na podaniu rozwiązań. Przygotowaliśmy ich aż 16 😍 Skupiamy się również jak możemy mierzyć to, czy nasze rozwiązania rzeczywiście pomagają.

Czeka na Ciebie również Inżynierska Prasówka z najnowszymi informacjami ze świata IT.

A więc w dzisiaj mam dla Ciebie:

Miłego czytania 😀


Inżynierska prasówka

Rozpoczynamy od LLM i tworzenia własnej aplikacji. I tempo nie zwalnia aż do tworzenia rozwiązań w oparciu o architekturę serverless. Bo pomiędzy: ataki bezpieczeństwa, przemyślenia o code review i zmiany 2024 w branży IT. Zapraszam 👇

  1. Commercetools store created by AI - Luciano Giavedoni
    Wielu z nas ma świadomość, że nowe LLMy potrafią tworzyć gotowe produkty. Ale wiedzieć i zobaczyć to na własne oczy to dwie odmienne kwestie.
    Luciano dzieli się swoją przygodą z najnowszym modelem Anthropica – Claude 3.5 Sonnet. I w ciągu 20 minut tworzy gotowe rozwiązanie e-commerce. Świetny pokaz możliwości współczesnego AI. Aż chce się zacytować: Future is now.
    https://www.linkedin.com/posts/kellygoetsch_commercetools-store-created-by-ai-sonnet-activity-7211699775824355328-qzpf/

  2. Polyfill supply chain attack hits 100K+ sites
    Osoby, które nawołują do dokładniejszego zarządzania zależnościami, mogą sobie dołożyć kolejny kamyczek do kolekcji.
    Nowy chiński właściciel popularnego projektu Polyfill JS wstrzyknął złośliwe oprogramowanie do ponad 100 tysięcy witryn. Wiele firm CDN pośredniczących w ruchu zmieniło odwołania do poprzedniej wersji, jednak ogromna liczba klientów została zainfekowana.
    Uważam, że w najbliższych latach będzie zwiększał się stopniowo nacisk na ciągły monitoring zależności, by szybko wykrywać tego rodzaju problemy.
    https://sansec.io/research/polyfill-supply-chain-attack

  3. Thoughts on Code Reviews 🔍- Luca Rossi
    Nie mogłem sobie odmówić wrzucenia tego materiału. 😉 Ale dla Luca warto.
    Opisuje wiele ważnych pytań dookoła Code Review, których sobie na codzień nie zadajemy. Jaki z tego zysk? W którym momencie przeprowadzać takie review? Jakie materiały przyśpieszają przeprowadzanie sprawdzania kodu?
    Dobry wstęp do naszych materiałów o PR Review.
    https://refactoring.fm/p/thoughts-on-code-reviews?r=1lusr

  4. What is Old is New Again – Gergely Orosz
    Większość newsletterów Orosza jest zamkniętych, więc nie linkuję do nich w moim biuletynie. Ale tym razem mamy wyjątek, a temat jest arcyciekawy.
    Gergely jest na tyle długo na rynku, że zauważa postępujący cykl koniukturalny. Stopy procentowe poszły do górę. I rynek IT wrócił do poziomu, na jakim był kilka lat temu.
    Znów mniejsze kwoty są przeznaczane na startupy. Gorszy rynek IT dla pracowników i dużo zwolnień. Firmy bezlitośnie pozycjonują profit ponad rozwój. Chyba trzeba się przyzwyczaić. 🤔
    https://newsletter.pragmaticengineer.com/p/what-is-old-is-new-again

  5. Refactoring to Serverless: From Application to Automation - Sindhu Pillai and Gregor Hohpe
    Zawsze miałem przeświadczenie, że serverless to nie tylko zmiana pisania kodu. Musimy zaadaptować nasz sposób myślenia o architekturze rozwiązania, infrastrukturze, modelach kosztowych, wykorzystaniu driverów architektonicznych.
    W ten mind-shift świetnie wpisuje się artykuł Sindhu i Gregora. Spójnie przedstawione zmiany jakie przynosi automatyzacja w kierunku serverless. De-facto wbudowujemy infrastrukturę w naszą aplikację. Jeszcze kilka lat temu było to dużym anty-patternem. Dziś, przez zmianę możliwości chmurowych, staje się świetną szansą na szybsze dostarczanie.
    Artykuł rozpoczyna się od trafnego stwierdzenia: “Serverless isn’t a runtime; it’s an architecture”
    https://aws.amazon.com/blogs/devops/refactoring-to-serverless-from-application-to-automation/


Jak usprawnić PR Review

Poprzedni odcinek newsletteru z Krzyśkiem poświęciliśmy problemom i powodom wystepowania PR Review. Nie chcemy jednak zostawić Ciebie bez rozwiązań. Nie chcemy jednak zostawiać Cię bez rozwiązań. Przychodzimy więc z dzisiejszą kontynuacją 😃

Możesz zauważyć, że spis rozwiązań zamieszczony poniżej jest bardzo długi. Jednocześnie trudno było nam uciec od takiej formy – większość praktyk jest ze sobą połączona. Np. nie wdrożysz Trunk-Based Development, jeśli nie masz odpowiedniego monitoringu. Niczym w serialu Dark: „Everything is connected”

IMG1.png

Rozpocznijmy jednak od innej strony. Aby wdrażać rozwiązania, trzeba najpierw wiedzieć co poprawiamy.

Jak mierzyć, czy jest lepiej

Nie chcemy tutaj przechodzić od razu do rozwiązań i proponować zmian. Jako pierwszy krok, warto zastanowić się:

  • Jakie problemy związane z PR Review zauważamy w naszym zespole?
  • Jak możemy zmierzyć obecny wpływ PR Review na nasz proces developmentu?
  • Jak chcielibyśmy by te miary wyglądały po zmianach?

To ma na celu:

  • Pokazanie jak obecnie wpływa na nas PR Review.
  • Zbudowanie koalicji dookoła zmian.
  • Określenie punktu docelowego, do którego chcemy dążyć.
  • Mierzenie postępów w działaniu.

Warto się nad tym wcześniej zastanowić – jeśli nie skwantyfikujemy zmiany, to osoby na podstawie własnego widzimisię mogą mówić, że jest gorzej. Nawet jeśli wcale nie będzie. Ich poprzednie doświadczenie z PR Review będzie ich pchało z powrotem do starych nawyków.

Szczególnie, że na początku rzeczy mogą rzeczywiście iść gorzej niż poprzednio. Jak to obrazował Seth Godwin w swoim poście Understanding Local Max:

IMG2.png

Na początku rezultaty będą gorsze. Trzeba się nauczyć pracy w parach, usprawnić CI/CD, kooperować z PM by zmniejszać rozmiar partii. To są ciężkie zadania, które dopiero z czasem procentują. Przygotujmy się na tygodnie a nawet miesiące zmieniania nawyków, obserwacji metryk i wprowadzania korekt.

Więc co mierzyć?

Poniżej spis tego, co możemy mierzyć. Ważna uwaga – nie chcemy mierzyć wszystkiego. Wybieramy to co uważamy za istotne dla naszego zespołu.

DORA – prędkość i jakość dostarczania

Warto rozpocząć od metryk DORA. Tutaj przypomnienie dla czytelników od Allstacks:

IMG3.png

Pomiar metryk DORA i klasyfikacja naszego zespołu (self-assessment) pozwoli nam szybko stwierdzić, gdzie jesteśmy teraz i określić, gdzie chcielibyśmy być.

  • Ponadto metryki DORA dzielą się na te powiązane z throughputem zespołu i jakością, więc powinniśmy dostrzec czy nie polepszamy jednego obszaru kosztem drugiego.

DevEx – odczucia pracy

Kolejnym obszarem, na który możemy zwrócić uwagę jest DevEx – Developer Experience. Z materiałami przychodzi tutaj Abi Noda i jego Applying the DevEx framework, jak i również ankiety Developer Experience od CTO firmy DevEX, Laury Tacho (link).

IMG4.png

Wspomnieliśmy wcześniej jak klasyczny PR Review wpływa negatywnie na obszary:

  • Flow (nagła i pilna dystrakcja w postaci review),
  • Feedback Loops (czekamy na review)
  • Cognitive Load (zmiana kontekstu, zbyt dużo zmian do przejrzenia w krótkim czasie).

Każdy z obszarów DevEx możemy próbować mierzyć i optymalizować.

Atrybuty jakościowe – jakość produktu

Metryki DORA mierzą prędkość procesów dookoła DevOps.

Można spojrzeć szerzej i skupić się na atrybutach jakościowych samego produktu. Jeśli je mierzymy to wdrażając usprawnienia do PR Review nasza jakość powinna wzrastać.

Poniżej spis kilkunastu z atrybutów, na które można zwrócić uwagę z ebooka Radka: Drivery architektoniczne w twoim zespole.

IMG5.png

W ramach zmian dookoła PR Review powinny poprawiać się wskaźniki takie jak:

  • Możliwość konserwacji – wcześniej zauważamy problemy dookoła utrzymania produktu.
  • Konfigurowalność – na etapie implementacji jesteśmy w stanie wyłapać, że nasze zmiany zamykają opcje dostosowania rozwiązania pod inne przypadki użycia.
  • Bezpieczeństwo – unikamy dziur bezpieczeństwa wdrażanych do oprogramowania.

A to już można kwantyfikować i oceniać.

Rozwiązania na Code Review

Czas na kilka propozycji rozwiązań.

Żadna z nich nie jest oczywiście silver bulletem na wszystkie omawiane przez nas problemy – zachęcamy, aby zastanowić się, które z nich występują w Twoim zespole i zbudować zgraną kompozycję z kilku- poniższych propozycji.

Pair programming

Rozwiązaniem najczęściej rekomendowanym przez popularnych mentorów takich jak Dave Farley czy Martin Fowler jest pair programming. Dla osób słyszących o programowaniu w parach po raz pierwszy polecam krótkie wprowadzenie z artykułu On Pair Programming ze strony Martina Fowlera:

IMG6.png

Pair programming essentially means that two people write code together on one machine. It is a very collaborative way of working and involves a lot of communication. While a pair of developers work on a task together, they do not only write code, they also plan and discuss their work. They clarify ideas on the way, discuss approaches and come to better solutions.

The Driver is the person at the wheel, i.e. the keyboard. She is focused on completing the tiny goal at hand, ignoring larger issues for the moment. The Navigator is in the observer position, while the driver is typing. She reviews the code on-the-go, gives directions and shares thoughts.

To co jest istotne z perspektywy PR Review to kwestia code review:

  • Pair programming liczy się tak jako approval, a programiści stają się ko-autorami rozwiązania również w kontroli wersji.
  • Tak przeprowadzone review jest również akceptowane przez reguły compliance i instytucje regulujące – informacja od Woutera Lagerweij.

Istnieje kilka zasad, które pomagają wprowadzić programowanie w parach do zespołu:

  • Warto rozpocząć od krótkiej sesji tłumaczącej złote zasady pair programmingu. Łatwo się krytykuje czyiś kod asynchronicznie. Kiedy robimy to na face-to-face to trzeba umieć wymieniać się spostrzeżeniami.
  • Warto eksperymentować z rotacją par i dedykowanym czasem na pair programming.
  • Znamy również dobrze działające przykłady z lekkim podejścia polegającym na ad-hoc synchronizacjach na slack huddle. Osoba chętna na dołączenie do sesji pair programmingu wchodzi na określony kanał dając sygnał reszcie zespołu, że można do niej dołączyć.
  • Pair programming dużo łatwiej wdraża się oczywiście w środowisku biurowym niż zdalnym. Do pracy zdalnej konieczne jest zastosowanie nowszych narzędzi. Na szczęście w ostatniej dekadzie powstało ich sporo (np. wtyczki do popularnych IDE)

Pair programming nie jest metodą, którą będziecie w stanie zastosować na 100% i w każdym momencie pracy. Błędem jest natomiast odrzucanie tej metody bez podjęcia jakiejkolwiek próby!

Non-blocking continuous code reviews

Klasycznie PR review dzieje się tuż przed mergem zmian. Z tego właśnie powodu pojawia się potrzeba, aby review był szybki:

  • Inny developer może być zablokowany bez tych zmian,
  • Biznes naciska, aby przeprowadzić szybko pierwsze testy i walidacje czy pomysł jest dobry.

Skuteczne zespoły świadome tego problemu dzielą scope rozwiązania na dwie sekcje:

  • Core - część zadania, która musi być dostarczona, aby odblokować biznes, robimy po niej release v1.0 - funkcja ma ograniczenia, może być schowana za feature flagą, może mieć błędy, kod może nie być najlepszy – ale biznes jest już w stanie weryfikować jej przydatność w praktyce.
  • Usprawnienia – rzeczy ważne, ale nieblokujące biznesu, dostarczamy je po release 1.0 w kolejnych wydaniach, tutaj często umieścimy skomplikowane reagowanie na rzadkie sytuacje, edge-case’y, usprawnienia jakościowe, security, wydajnościowe, dodatkowe testy.

Jeżeli pomyślimy o jakości kodu w oprogramowaniu zawsze można coś usprawnić, ulepszyć, poprawić, zmienić. Kod nigdy nie jest skończony, nigdy nie mamy pewności, czy nie ma w nim błędów, edge case’ów.

Idąc za tą teorią, PR review skupiony przede wszystkim na poprawianiu jakości i dzieleniu się wiedzą również nie musi być etapem blokującym dla biznesu. Jeżeli wyobrazimy sobie “code review” jako kolumnę w kanbanie tuż przed releasem rozwiązania, to teraz zostaje ona przeniesiona na moment po release.

Jak to działa w praktyce?

  • Zamiast przeprowadzania review przed każdym PR, zespół spotyka się na kilka godzin raz w tygodniu i przegląda kod razem.
  • Można eksperymentować ze spacerowaniem po codebase poprzez IDE i dyskusje na temat ostatnich epików, zamiast ograniczać się do zmian commit po commicie.
  • Wykonujemy review patrząc na kontrolę wersji lub korzystając z narzędzi wyspecjalizowanych w tym celu takich jak np. JetBrains Space, Atlassian Crucible, ex-remit, reviewboard.
  • Warto poświęcić trochę czasu na spojrzenie na epiki, które wdrożyliśmy kilka tygodni temu i zweryfikowanie, czy nasze hipotezy się sprawdziły - może ostatnie założenia dotyczące architektury i abstrakcji są niepoprawne?

Zauważ, że taka regularna sesja pozwala nam wejść w zagadnienia jakości znacznie głębiej. Z tego typu spotkania powinniśmy wrócić z szeregiem pomysłów na usprawnienia. Niewykluczone, że zajmą one wiele dni, ale biznes nie jest tym aż tak rozgoryczony, bo otrzymał już rozwiązanie, z którym może w międzyczasie eksperymentować.

Czy to jest realne do wprowadzenia? Thierry de Pauw opisuje takie podejście w swoim artykule Non Blocking Continuous Code Reviews, a case study i prezentacji na tej podstawie:

IMG7.png

Podejrzewamy, że zastanawiasz się - ale co z funkcją PR review polegającą na wyłapywaniu błędów i znaczących nieporozumień, rzeczy poza samą jakością kodu? Jak ograniczyć ryzyko? Proponujemy zmixować tą metodę z innymi zaproponowanymi w tym poście 😊

Przykładem na zastosowanie cyklicznego sprawdzania kodu przed wdrożeniem dzieli się w swoim artykule Dev Carousels Maciej Jędrzejewski:

Besides swarming, pair and mob programming and other things that I described in this article, there is still a place for something called a Dev Carousel.

This is a regular, as flexible as possible, daily meeting attended only by technical people where we talk about technical issues related to architecture, structure, problems, patterns, strategies, implementation, and many, many more. Like coffee and cars :)

Spotkania te odbywają się z zasady rano, trwają od 30 minut do godziny i uczestniczą w nich wyłącznie osoby techniczne. Omawiane są kwestie dotyczące architektury, struktury kodu, wzorców projektowych oraz bieżących problemów i strategii.

Spotkania te mają na celu skrócenie czasu przetwarzania pull requestów. Sprawdzamy kod na bieżąco, co prowadzi do poprawy jego jakości oraz zwiększenia efektywności pracy zespołu. Na końcu mamy kod sprawdzony, czyli gotowy do wdrożenia.

Maciej podkreśla również, że Dev Carousel sprzyja integracji zespołu, tworząc przestrzeń do wymiany doświadczeń i wiedzy technicznej. Regularność i elastyczność tych spotkań pozwalają na bieżąco monitorować stan projektu.

Ship / Show / Ask

Możemy również spojrzeć na ten problem z zupełnie innej perspektywy. Jak często zdarza nam się angażować PR code review do zmiany jedno-wyrazowego typo, zmiennej czy innego buga? Czy w takiej sytuacji blokada, review i approval zawsze są niezbędne? Na pewno są sytuacje, gdy taki fix jest wart podzielenia się, ale z naszego doświadczenia wynika, że często robimy to tylko dla formalności.

Rouan Wilsenach proponuje metodę Ship / Show / Ask, która pomaga nam ograniczyć niepotrzebne PR review i wdrożyć ten proces stopniowo:

Ask: Pytaj o review, potem merguj

IMG8.png

To jest nasza klasyczna metoda PR review, którą staramy się ograniczyć, ponieważ jest kosztowna.

Jak i kiedy ją stosować?

  • Pytaj o review celowo, a nie losowo i przez automat – to kosztowna metoda, więc stosujmy ją odpowiedzialnie, wymaga Twojego self-review i przygotowania.
  • Pytaj o konkretne obszary, szczególnie te związane z większym ryzykiem czy niepewnością.
  • Kiedy dotykasz kodu, którego nie znasz, jest bardzo skomplikowany, wpływa na wiele obszarów albo doprowadzi do trudno odwracalnych zmian.
  • Nie proś o approval, tylko o sprawdzenie jakości.

Show: najpierw merguj, potem pokaż i poproś o feedback

IMG9.png

Szukaj okazji kiedy możesz zbudować zaufanie poprzez pokazanie, że nic złego się nie stało - jesteś w stanie dowieźć szybko rozwiązanie, ale jednocześnie tworzysz pole do dyskusji, natomiast nie jest ona dla nikogo blokująca. Świetnie się sprawdza do dzielenia się wiedzą.

Jak tę metodę stosować?

  • Pokaż interesujący, nietypowy błąd już po jego naprawieniu.
  • Przedstaw ciekawe rozwiązanie dla reszty, które jest FYI a nie blokerem.
  • Poproś o feedback na ewentualny refactor czy optymalizację, którą można wdrożyć chwilę później.

Ship: po prostu merguj

IMG10.png

Wprowadzasz zmianę natychmiastowo bez zakłócania pracy innych osób, najlepiej w sytuacjach, gdzie review jest wyłącznie formalnością.

Kiedy ją stosować?

  • Naprawienie niewielkiego błędu, typo, przeoczenia - czegoś oczywistego, co nie wymaga specjalnej konsultacji
  • Dodanie prostej akcji CRUD’owej zgodnie z zasadami opracowanymi przez zespół, nic nowego, mamy dużą pewność tej zmiany
  • Aktualizacja dokumentacji albo pliku konfiguracyjnego dla dokumentacji
  • Usprawnienia w oparciu o feedback reviewera
  • Dodanie brakującego testu do już istniejącego suite’a
  • Zastosowanie prostej zasady scoute’a gdy zauważyliśmy ku temu potencjał
  • Kosmetyczne zmiany na UI
  • Wiele firm eksperymentuje też w ten sposób z małymi aktualizacjami zależności

Mogłoby się wydawać, że zmian typu “Ship” i “Show” powinno być mało, ale w praktyce jest ich całkiem sporo. W swoim artykule Rouan przedstawia kilka dodatkowych zasad i sugestii dla zespołów, z którymi warto się zapoznać.

Luca Rossi opublikował też niedawno artykuł o bliźniaczej metodzie “Automate / Defer / Pair”, w którym twierdzi, że w zdrowej organizacji najwięcej zmian powinno być typu “non-blocking review” w postaci “Defer”. Co ciekawe, alternatywą dla “Ask” w jego formule jest “Pair”… co dobrze łączy się z innymi pomysłami w naszym poście.

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:

IMG11.png

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:

IMG12.png

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.

IMG13.png

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 niżej 😉) wymieniają TBD jako niezbędny pierwszy krok w kierunku codziennego wdrażania;

Rozwiązania na zachowanie jakości

Argumentem strony pro PR Review jest to, że taki model sprawdzania kodu zapewnia jakość.

Przygotowaliśmy więc praktyki, które pozwolą wam zachować jakość pomimo braku PR Review.

Continuous Integration

Możesz teraz powiedzieć: „Jak to, przecież narzędzie CI nie zapewni mi jakości”. I masz dużo racji. Ale jednocześnie się mylisz 😃

Nie chodzi nam tutaj o narzędzie (Jenkinsa, GitHub Actions, czy inne). Chodzi nam o praktykę Continuous Integration. Najlepiej wykorzystać tutaj fragmenty z ksiązki książki Continuous Delivery autorstwa Jez Humble i David Farley:

1. Szybkość integracji

The most important practice for continuous integration to work properly is frequent check-ins to trunk or mainline. You should be checking in your code at least a couple of times a day.

Integrujesz swój kod z głównym branchem przynajmniej raz dziennie. Wielkość twoich zmian jest na tyle mała, że kodu do sprawdzenia nie jest dużo. A to zmniejsza diametralnie liczbę potencjalnie wprowadzanych błędów.

2. Testy potwierdzające działanie

If you don’t have a comprehensive suite of automated tests, a passing build only means that the application could be compiled and assembled. It’s essential to have some level of automated testing to provide confidence that your application is actually working.

Uruchamiasz zbiór testów co integrację. Testy zapewniają, że twoja zmiana nic nie popsuła. Jeśli testy przechodzą, to wiesz, że jesteś gotowy by wrzucać na proda. A jeżeli nie ufasz swoim testom, bo są niedeterministyczne, musisz najpierw rozwiązać ten problem.

3. Build jest krótki i zawsze działający

Ideally, the compile and test process that you run prior to check-in and on your CI server should take no more than a few minutes, ten max. If the build breaks, the developers responsible are waiting to fix it. They identify the cause of the breakage as soon as possible and fix it.

Jeśli kod buduje się zbyt długo to naprawiacie build. Jeśli proces budowania się zepsuł to priorytetem zespołu jest naprawienie właśnie tego elementu. Aktywnie pracujesz, aby budowanie kodu dawało natychmiastowy feedback. Wg guide’ów XP szybki build to taki który trwa mniej niż 10 minut łącznie z testami.

4. Lokalne środowisko pracy

Developers should always work from a known good starting point when they begin a fresh piece of work. They should be able to run the build, execute the automated tests, and deploy the application in an environment under their control.

Środowisko pracy wspiera Cię, umożliwiając lokalne przeprowadzanie wszystkich testów. Posiadasz odpowiednią konfigurację i pliki startowe. Jesteś w stanie lokalnie odtwarzać błędy i testować zmiany.

Włączając w swoją pracę aspekty wymienione wyżej, zyskasz mechanizmy integracyjne, które umożliwią uniknięcie części problemów jakościowych dookoła PR Review.

Definiowanie i wdrażanie przypadków testowych

Sprawdzanie przypadków testowych już po napisaniu kodu sprawia, że przywiązujemy się do tego co napisaliśmy - skupiamy na rozwiązaniu, a nie na problemie. Co sprawia, że trudniej jest wyłapać, czy aby na pewno pokryliśmy wszystkie przypadki testowe.

  • Oto, co może tutaj pomóc:BDD – Behavior-Driven Development - projektowanie w oparciu o scenariusze użycia, jakie użytkownik oczekuje podczas interakcji z aplikacją.
  • TDD – Test-Driven Development – rozpoczynanie od napisania testów, a dopiero później przejście do napisania funkcji, która spełni testy.

Zastosowanie tych obu technik wymusza:

  • Myślenie przypadkami użycia i przypadkami brzegowymi (piszemy lepszy interfejs).
  • Wbudowywanie jakości w pisany kod od samego początku.
  • Zapewnienie, że nasz kod zawsze ma testy, przez co ufamy CI/CD.

Dzięki temu wcześnie wykrywamy problemy i redukujemy, lub nawet eliminujemy potrzebę ręcznych przeglądów kodu. Testy są uruchamiane automatycznie i natychmiastowo informują o problemach, co pozwala na szybką reakcję i korektę.

Ciekawie to podsumowuje Shormistha Chatterjee w swoim artykule o BDD vs TDD:

IMG14.png

  • BDD – Build the right thing.
  • TDD – Build the thing right.
  • Both – Test first

Systematyczne i powtarzalne testy są bardziej efektywne i dokładne niż ręczna weryfikacja.

Z doświadczenia wiemy, że TDD jest trudne dla wielu developerów, ale jak wszystkie praktyki wymaga przećwiczenia. Często problemem są nasze luki w wiedzy dotyczącej właśnie pisania testów automatycznych i to, że zajmuje nam to więcej czasu niż powinno. Nie przejmuj się jednak – po paru tygodniach pracy w TDD ten problem znika.

Statyczna analiza kodu

Naszym celem jest, aby automatycznie potwierdzać wysoką jakość kodu i wymagane standardy zamiast wymuszać tą aktywność na kolegach z zespołu.

Co możemy wykorzystać?

  • Lintery kodu - sprawdzamy kod pod kątem zgodności ze zdefiniowanymi standardami stylu i składni. Unikamy więc nit-pickingu podczas review. Narzędzia to np. ESLint czy StyleCop.
  • Analiza jakościowa - zagłębiamy się w logikę i strukturę kodu, aby sprawdzić określone standardy jakości i bezpieczeństwa. Narzędzia to np. Sonarqube czy Coverity.
  • Testy zależności – potwierdzamy, że wszystkie zewnętrzne biblioteki i moduły, na których opiera się projekt, są aktualne i bezpieczne. Narzędzia to np. Dependabot, czy Renovate.
  • Standardy architektury - weryfikujemy zgodność ze standardami architektonicznymi, nazewnictwo klas, strukturę kodu.

Dzięki temu odpada nam duża część manualnej pracy podczas review. Spory kawałek zapewniania jakości przejmuje automat. Idealnie, jeżeli większość tej analizy możemy wykonać na lokalnej maszynie tak, aby nasz feedback loop był jak najkrótszy.

Monitoring i prewencja

Wraz ze zwiększeniem prędkości dostarczania i wzrostem złożoności systemu, część zapewniania jakości musimy przenieść na stronę post-wdrożeniową. A w tym pomaga nam obserwowalność.

Zamiast inwestować w kosztowne i czasochłonne testy E2E (które również zmagają się z problemami niedeterministyczności), skupiamy się na wdrażaniu mechanizmów obserwowalności. Wielkie firmy technologiczne, takie jak Netflix czy Amazon, już od dawna polegają na zaawansowanej obserwowalności zamiast na tradycyjnych metodach testowania. W środowiskach mikroserwisowych trudno o inną formę obrony.

Dziś skupiamy się na 2 aspektach obserwowalności – monitoringu i prewencji.

Monitoring, jak sama nazwa wskazuje, skupia naszą uwagę na monitorowaniu kluczowych scenariuszy naszego użytkownika. Dzięki niemu szybko wykrywamy problemy, zanim staną się krytyczne. Jesteśmy powiadamiani natychmiast o anomaliach i odchyleniach od normy.

Gdy już zaobserwujemy problemy mamy do dyspozycji szereg mechanizmów prewencji:

  • Wycofujemy zmian (manualnie lub automatycznie), gdy określone wskaźniki jakościowe się zmniejszą.
  • Wyłączamy scenariusz dla danego zbioru użytkowników bądź całkowicie (za pomocą feature flag).
  • Zmieniamy dynamicznie konfiguracje dotyczące bezpieczeństwa na bardziej restrykcyjne, gdy spodziewamy się ataku lub autoskalujemy w sytuacji, gdy wzmożony ruch jest uzasadniony.
  • W sytuacjach, gdy powyższe działanie nie jest możliwe, możemy automatycznie eskalować alert do zespołu na dev support duty.

Mechanizmy obserwowalności redukują potrzebę ręcznych przeglądów i testów, ponieważ system sam wykrywa i sygnalizuje potencjalne problemy. Automatyczne mechanizmy prewencyjne minimalizują ryzyko błędów, jednocześnie obniżając koszty związane z manualnymi procesami weryfikacji.

Nowoczesne firmy inwestują następnie w chaos engineering, aby zaobserwować czy infrastruktura, monitoring i automatyczna prewencja są w stanie poradzić sobie w stresujących sytuacjach. Takie “testy” dają nam znacznie większą pewność jakości i niezawodności niż testy e2e.

Więcej o tym opisał Radek w newsletterach:

Rozwiązania na zwiększenie zaufania

Jednym z głównych powodów przeprowadzania PR Review nie są problemy funkcjonalne, ale emocjonalne. Nie ufamy naszym kolegom/koleżankom, że wypełnią swoje obowiązki jak należy. To sprawia, że chcemy sprawować bezpośredni nadzór nad ich pracą.

Praktyki opisane poniżej adresują te problemy – chcemy zwiększyć zaufanie do naszego zespołu, tak by powierzać im określone obszary produktu,

Główną metodą, którą możemy tutaj polecić jest wspomniane już Ship / Show / Ask i przejście więcej na tryb “Ship” i “Show”. Istnieje jeszcze kilka praktyk wartych polecenia:

Techniczne Show & Tell

Podejście Show & Tell często pojawia się dookoła metod zwinnych. Jest to metoda na zaangażowanie interesariuszy w wyniki sprintu – tak, aby przedyskutować to co zostało dowiezione i ustalić dalsze akcje.

To samo podejście możemy zastosować do technicznej pracy.

  • Wykorzystujemy Ship / Show / Ask i zbieramy materiały w trakcie sprintu.
  • Spotykamy się +/- raz per sprint, po jego zakończeniu.
  • Przegadujemy to, co zostało wdrożone, jak działa, jakie są wyniki.
  • W ramach tego spotkania podejmujemy się review kodu, który jest już wdrożony.

Takie spotkanie skupia się na cyklicznej wymianie wiedzy i uczeniu od siebie wzajemnie. Dzielimy się dobrymi praktykami i jednocześnie adresujemy problemy, z którymi będziemy musieli sobie poradzić w kolejnych tygodniach.Sama dyskusja ma charakter o wiele głębszy niż typowe PR Review. Do Technicznego Show&Tell możemy bardziej się przygotować. Każdy ma na to spotkanie czas. Mamy miejsce na otwarte pytania.

Dzięki temu:

  • Poznajemy punkt widzenia innych osób.
  • Rozwijamy kompetencje kolegów i koleżanek o kluczowe praktyki
  • Łatwiej adresujemy palące kwestie w zespole.

Ostatecznie, zaufanie wzrasta.

Określenie ważności systemów i obszarów

Wymagania jakościowe nie są takie same dla wszystkich systemów i obszarów:

  • Dojrzały system posiadający 10k aktywnych użytkowników będzie miał inną tolerancję na błędy i fuckupy niż startup mający 10 użytkowników.
  • Inaczej podejdziemy do tempa rozwiązywania problemów w projekcie z 99.99 SLA, a inaczej w wewnętrznej aplikacji w trybie maintenance, utrzymywanej przez jedną osobę na part-time.
  • Inne oczekiwania jakościowe mają prototypy rozwiązań, co do których ani my ani biznes nie ma pewności, bo budujemy je pierwszy raz. Inne, kiedy robimy refactor czy rewrite i wiemy, jak oczekiwania spotkały się z rzeczywistością (poziomy ryzyka i niepewności są zupełnie inne).
  • Moduł będący naszą główną domeną biznesową będzie miał inne założenia co do oczekiwanej długości życia kodu niż moduł będący domeną generyczną. Tę domenę być może zastąpimy w przyszłości usługą third party albo zoutsourcujemy dla innej firmy. Dlatego powinniśmy mieć inne oczekiwania co do pokrycia testami, niezawodności, dokładności monitorowania, architektury.
  • I tak dalej…

Z wyżej wymienionych względów, utrzymywanie tego samego standardu jakości i kodu nie jest optymalnym inżynieryjnie rozwiązaniem. Zastanów się, czy praktyki, które stosujecie są dopasowane do fazy i znaczenia tego projektu. Czy każdy w zespole zdaje sobie z tego sprawę?

Zadbaj, aby te praktyki były w zespole wiążącym elementem wzajemnego zrozumienia – niczym kontrakt. Niech każdy wie, co jest od niego oczekiwane, czemu w tym projekcie jesteśmy bardzo dokładni, a w innym mniej. Zobaczysz jak wiele rozbieżności znajdziesz pomiędzy poszczególnymi członkami zespołu.

Mając ustalone jasne reguły łatwiej jest sobie ufać, bo każdy wie co jest od niego oczekiwane.

Unikanie matkowania

Ciekawym i zupełnie nieoczywistym problemem jest “matkowanie” mniej doświadczonych programistów.

Świetni liderzy, mając dobre intencje wyjątkowo dokładnie weryfikują zmiany mniej doświadczonych programistów. Jest to forma mentoringu i nauki. W sytuacjach ekstremalnych może jednak dojść do sytuacji, gdzie zespół polega na liderze jako na swojej “siatce bezpieczeństwa”, trochę jak na kolejnej warstwie testów.

Te zachowania są często nieświadome:

  • Nie mam czasu zrobić dokładnego self-review, ale lider na pewno złapie, jeżeli mam tam jakiś błąd (lenistwo).
  • Jeżeli lider jest z tym OK no to na pewno jest dobrze (abdykacja ownershipu).
  • Lider też nie wyłapał tego błędu (brak odpowiedzialności za własne błędy).

Tworzy to ogromny bus factor w zespole, wywiera presję na liderze i przyczynia się do powstania dysbalansu wysiłku.

Jeżeli jesteś takim liderem – chociaż to zabrzmi kontrowersyjnie – przestań ratować swój zespół. Gdy zabraknie siatki, wielu z nich szybko dojrzeje i podniesie swoje standardy.

Trial by fire

Często w naszych firmach zakładamy, że bardziej doświadczona osoba zawsze musi patrzeć mniej doświadczonej na ręce.

Jeżeli jednak nie jesteś w stanie usunąć wymogu approvala od członków zespołu nawet po okresie 6 miesięcy, zastanów się, dlaczego tak się dzieje:

  • Co nowa osoba musiałaby zrobić, aby zyskać Twoje zaufanie?
  • Czy brakuje jej jasno sprecyzowanych oczekiwań, albo ścieżki rozwoju?
  • Co najgorszego może się stać, jeżeli jednak na to pozwolisz?
  • Jak szybko uda się tą najgorszą sytuację odwrócić?

Z naszego doświadczenia firmy, które mocno unikają konfrontacji dochodzą do poziomu „ruinus empathy” z Radical Candor:

IMG15.png

Podejście „trial by fire” promuje przeciwne zachowanie

  • Aby móc wrzucać kod na produkcję, musisz otrzymać odpowiednią liczbę “approvali” dotyczących Twojej jakości kodu od bardziej doświadczonych kolegów.
  • Każdy kto przejdzie ten okres “próbny” staje się zaufany – może wdrażać bez approvala. Razie trudnych sytuacji sama zgłosi się do reszty zespołu o radę.
  • To podejście warto połączyć z określeniem podziału ważności obszarów i systemów – by określać w których miejscach mamy takie prawa.

Jest to prosta zasada i łatwa do wdrożenia w założeniach funkcjonalnych, ale trudniejsza w emocjonalnych.

Dlatego upewnij się, że ta obawa o wrzucanie zmian przez twoich kolegów i koleżanki jest uzasadniona - że nie jest to tylko Twój własny strach lub ego.

Podsumowanie

To już wszystko na dzisiaj – ten artykuł jest już i tak za długi. 😅

Na tym etapie pewnie dostrzegasz już, że wiele z tych praktyk się uzupełnia, ma części wspólne lub nawet bardzo podobne założenia. PR Code review jest głęboko zakorzeniony w kulturze większości firm a dla wielu developerów odejście od tego procesu wydaje się być wręcz obrazoburcze 😊.

Mamy nadzieję, że po lekturze rozumiesz, że istnieją lepsze alternatywy. Zachęcamy do eksperymentów w Twoim zespole i zmierzenia sytuacji przed i po w oparciu o metryki powyżej - a potem wróć do nas, i pochwal się jak Ci poszło!


📧 Prześlij dalej

Dzięki, że doczytałeś(aś) do końca. 😊

Wszystkie poprzednie wydania newslettera są dostępne tutaj.

Jeśli spodobał Ci się mój newsletter, prześlij go proszę osobom, którym też mógłby się spodobać. Z góry dziękuję.

A jeśli nie jesteś jeszcze w newsletterze, to zachęcam do zapisania się.

Polecam się na przyszłość!
Radek Maziarka

Radek Maziarka
"Inżynierskie podejście do produktów cyfrowych."

P.S. Co myślisz o tym newsletterze? Odpisz :)