Napisao: Ivan Lučin, VP of Engineering u Productiveu

Nedavno sam naišao na članak na blogu firme Arkency o nedostacima pull requestova. Brzo sam shvatio da je riječ o zanimljivoj temi koju vrijedi podijeliti s kolegama, te sam odmah počeo pisati članak u kojem analiziram svaku tvrdnju iznesenu u tekstu.

Proces pregledavanja pull requestova (PR review) bazira se na tome da svaku promjenu koju jedan developer napravi, neka druga osoba pregleda i ostavi komentare u pull requestu, upućujući na potencijalne probleme ili neka moguća bolja rješenja.

Kod nas u Productiveu, inženjeri na dnevnoj bazi pregledavaju pull requestove, što nam pomaže kod:

osiguravanja kvalitete i stabilnosti koda
dijeljenja domenskog znanja unutar tima
organizacije mentorskih aktivnosti

Svaki organizacijski proces uvodi svojevrsne komplikacije i probleme, pa tako i ovaj, ali smatram da se isplati uložiti u proces pregledavanja pull requestova.

U ovom ću osvrtu analizirati tvrdnje iz ranije spomenutog članka i iznijeti svoje mišljenje o njima. Pa krenimo o nedostacima pull requestova!

1. Dugoživući pull requestovi dovode do “merge” konflikata

Ovo je istina, međutim, to ne bi smio biti problem ako se prakticira dobra distribucija rada u timu.

Većina proizvoda sastoji se od niza modula, npr. različitih ekrana ili dijelova aplikacije. Rad na tim modulima se uvijek treba raspodijeliti na način da pull requestovi ne utječu međusobno jedni na druge.

Ako projekt nije dobro modulariziran, postoji nekoliko praksi kako bi se izbjegao ovaj problem:

velike promjene treba uvijek razbijati na manje, ali funkcionalne PRove
PR treba redovito ažurirati s novim promjenama iz glavnog codebasea (ja ovo radim prije svakog pregleda tuđeg pull requesta).
pripremni refactoring uvijek treba odraditi prije započinjanja sa novim funkcionalnim promjenama

Najčešća greška koju programeri rade je da ulažu previše truda i vremena u jedan PR, čime se znatno otežavaju procesi pregledavanja i prihvaćanja promjena u glavni branch.

2. Preglednost izmjena smanjuje se s veličinom pull requesta

Developeri često upadaju u ovu zamku. Tok misli im je “Refaktorirat ću ovo kad sam već tu”, ili “Trebao bih ovo dovršiti do kraja prije slanja na pregled”. Ovakav pristup dovodi do prevelikih PRova i sporog procesa razvoja.

Tri pull requesta sa 100 linija koda će se pregledati i odobriti brže od jednog s 300 linija koda.

Naravno, nisu sve promjene iste. Na primjer, 100 linija koda koji je izgeneriran od strane frameworka nije pretjerano kompleksan. Pri testiranju je potrebno napisati puno linija koda, ali bi se one trebale i lako čitati. Isto tako, uvijek je lakše shvatiti kod s manje zavisnih dijelova.

U načelu, što je osoba bolje upoznata s projektom, tim više promjena smije ubaciti u pojedinačni pull request. Programeri s manje iskustva trebali bi raditi čim manje PR-ove.

3. Pull requestovi usporavaju vrijeme razvoja i time smanjuju užitak posla

U programiranju nema ništa uzbudljivijeg od brzog isprobavanja rješenja za neki problem koji vam je prethodno razbijao glavu. Izvorna tvrdnja autora je da proces pregledavanja pull requestova uništava tu brzu povratnu informaciju, te je posljedično sam posao programiranja manje zabavan.

Međutim, ako je to istina, to znači da:

programerski tim kreira velike i prekomplicirane PR-ove (već smo ustanovili da ovo treba izbjegavati)
ne postoji dobar proces testiranja promjena na PR-u u pravom okruženju
promjene nisu napravljene u dobroj izolaciji od ostalih funkcionalnosti

Nije nužno pull request uvijek pregledavati u potpunoj izolaciji od live okruženja. Jednostavno možete deployati kod na testno okruženje, ili čak i na produkcijsko, ali na zasebnu domenu tako da samo vi i vaš tim možete testirati promjene.

U Productiveu, za našu frontend aplikaciju imamo automatizirani proces za deployanje PR-ova. Kad na Githubu kreirate novi PR, ili pak mijenjate postojeći, okida se proces Github action koji podigne kod i instancu aplikacije na testno okruženje.

Ova akcija uzima naziv git brancha i objavi frontend aplikaciju na subdomenu s istim nazivom, npr. https://cool-new-stuff.review.productive.io.

Voilà, sad možete isprobati promjene iz PR-a uživo, dvije minute nakon što ste kreirali PR.

Uz to, dodatni Github action upiše komentar s poveznicom na testno okruženje, tako da je ne morate ručno upisivati u URL tražilicu browsera.

Postoji i drugi mehanizam za ubrzavanje procesa testiranja promjena, a koja je sasvim sigurna ako se ispravno odradi. Pomoću tzv. “feature flags” možete osigurati da pull requestovi ne naprave nikakvu štetu te da se novi kod implementira odvojeno od postojećih funkcionalnosti.

Feature flags vam omogućavaju da isprobate nove promjene u izoliranom opsegu korisnika, bez da išta mijenjate na ostatku sustava. Opseg u kojem testirate nove funkcionalnosti može biti samo jedan korisnik, jedan račun ili samo jedan API poziv. Feature flags su moćan mehanizam koji naš tim svakodnevno koristi. Omogućava izmjene na stvarnom okruženju brzo i bez opasnosti od nepredviđenih grešaka.

4. Proces pregledavanja pull requestova je često površan

Programeri često podcjenjuju koliko je pažnje i vremena potrebno uložiti u kvalitetno pregledavanje pull requestova. No, ako se pregled pull requesta odradi površno, cijeli proces gubi smisao.

Ako se PR čini prekompliciran – zašto ne raditi pregled PR-a u paru s osobom koja je napisala kod?

Programiranje u paru (pair programming) je korisno za mentoriranje junior developera. Međutim, u mnogim drugim slučajevima je gubitak vremena zato što ne rješavate kompleksne probleme cijelo vrijeme.

Bolji način rada je pregledavanje PR-a u paru s autorom. U tom slučaju autoru prepuštate da napravi inicijalni posao i objasni svoj pristup rješavanju problema, a zajedničku raspravu vodite samo oko kompleksnijih pitanja.

Nadalje, često se događa da programeri osjete kad nešto s njihovim rješenjem nije sasvim u redu.. U tim slučajevima korisno je biti iskren i ostavljati komentare u PR-u svugdje gdje smatraju da se nešto potencijalno može napraviti bolje i drugačije. Time se značajno pojednostavljuje proces pregledavanja PR-ova.

5. Komentari na PR-u često blokiraju “merge”, a ne bi trebali

Do ovih situacije često dolazi kada manje iskusni developeri pregledavaju pull requestove. Osoba koja komentira na PR-u bi uvijek trebala jasno reći autoru blokira li komentar merge ili ne. Također, nije dobro previše inzistirati na sitnicama i problemima koji ne blokiraju.

Moj osobni pristup je takav da kada radim s manje iskusnim developerima, uvijek im ukažem na pogreške te im u isto vrijeme i pojasnim rješenje. Kada je riječ o greškama koje ne blokiraju merge, reći ću im da na to pripaze u sljedećem PR-u. U konačnici sam ja taj koji radi merge PR-a, tako da se oni sami ne moraju brinuti je li PR dovršen ili ne.

Kad pregledavam PR-ove iskusnijih developera, reći ću im što mislim o problematičnim mjestima u kodu, ali najčešće ću odobriti promjene i prepustiti autoru da odradi ispravke na način koji žele.

Važno je razumjeti da osoba koja ostavlja komentare na PR-u nije uvijek u pravu. Autor treba donositi odluke po pitanju komentara jer, u konačnici, autor je taj koji je kreirao kod.

6. Zbog PR procesa developeri sporije razvijaju osjećaj odgovornosti

Ova je tvrdnja je dosta važna, no može li si organizacija dozvoliti probleme u proizvodu samo kako bi developeri naučili odgovornosti? Ponekad se to isplati, ali u većini slučajeva to nije poželjno.

Stabilnost koda nije jedini razlog zašto je dobro raditi pregledavanje pull requestova. Tu su također i mentoriranje, dijeljenje znanja, bolja organizacija objavljenih verzija koda, potpunost funkcionalnosti, strukturirana diskusija o promjenama i razlikama u PRu, itd…

U Productiveu potičemo ljude da sami mergeaju PR-ove bez pregleda druge osobe kad vjeruju da su promjene u redu. Developer treba imati slobodu to napraviti u dijelu koda gdje on vodi razvoj. Vjerujem da se ovime može trenirati odgovornost. Naime, kad ljudi nešto izgrade iz temelja, osjećati će i odgovornost da dugoročno sve ostane kako treba.

7. Pull requestovi nas odvraćaju od kontinuiranog refaktoriranja

PR-ovi će usporiti refaktoriranje. Kad shvatite da je refaktoriranje potrebno, trebate se prebaciti na drugi git branch kako biste refaktoriranje zasebno implementirali, poslali PR na pregled, pričekali odobrenje i spojili izmjene u glavni kod.

Međutim, po mojem je mišljenju ovo još uvijek bolja opcija. Pregled pull requestova koji samo radi refaktoriranje ne bi trebao biti kompliciran, tj. ne bi smjelo doći do promjena u logici već samo u strukturi koda. Ovo je osobito točno ako ste dobro pokriveni testovima. Ako niste, možda biste trebali prvo napisati testove?

Također, možda takav PR uopće ni ne treba proći proces pregleda ako je sve trivijalno. Ako pak treba, spomenite osobi koja pregledava PR da je riječ o PR-u samo za refaktoriranje koji blokira razvoj nove funkcionalnosti, tako da se taj posao prioretizira.

Dobrom timskom komunikacijom može se minimizirati većina problema u cijelom procesu pregledavanja pull requestova.

8. Pregledavanje pull requestova stvara negativne emocije u timu

Postupak pregledavanja pull requestova ponekad može kreirati više negativnih emocija no što je potrebno. Odgovornost leži i na autoru i na recenzentu, a oni se “samo” trebaju pridržavati jednog jednostavnog pravila:

Ne budi seronja kad pregledavaš ili pišeš pull request.

Ukratko, izvorni članak tvrdi da bismo trebali pustiti developerima da rade što žele jer bi se mogli uvrijediti zbog komentara na njihovim PR-ovima. To je smiješno i neozbiljno. Ako vaši inženjeri ne znaju pristojno komunicirati, u timu će postojati i veći problemi od developera uvrijeđenih zbog komentara na pull requestu.

Zaključak

U izvornom članku nalazi se popis prijedloga kako poboljšati proces rada s pull requestovima u vašem timu. Mahom su to izvrsne ideje te vas potičem da ih isprobate. Hvala Arkencyu na odličnom i poučnom članku!

Kao i sa svim drugim organizacijskim procesima, ne trebamo ih doživljavati preozbiljno jer ponekad mogu stvarati probleme te nas usporavati. Trebali bismo biti pragmatični oko pregleda PR-ova te ih preskočiti kad smatramo da otežavaju rad.

Nemojte se slijepo držati pravila, razmišljajte vlastitom glavom i napravite pravu stvar ovisno o situaciji.

I mala napomena za kraj – imajte na umu da su međusobna obzirnost i uljudnost pri pregledavanju i komentiranju koda ključni za dobro funkcioniranje inžinjerskog tima i stvaranje kvalitetnog proizvoda.