Zavádění code review 3/5: Naše provedení

V minulém díle jsme si nastínili obecné výhody code review. Tentokrát si povíme, jak přesně jsme proces code review ustanovili u nás v Trigamě ve webovém týmu.

 

Mějte na paměti, že každý tým je unikátní, řeší různé problémy (a má tudíž i různé cíle), má k dispozici různé podpůrné nástroje, různé množství použitelné kapacity zúčastněných a v konečném důsledku je nutné počítat také s tím, že jednotliví lidé jsou rozdílní. Také je třeba poznamenat, že každý tým řeší různé projekty, jejichž citlivost vůči případným chybám je velmi různorodá. Proto je každý code review proces unikátní. To, jestli je „správný“, je dáno tím, nakolik vnímá námi stanovené cíle jako vhodné řešení problémů. Tedy jinak řečeno, na internetu nenajdete, jak má vypadat správný code review, spoustu otázek si budete muset zodpovědět sami.

Jednotlivé problémy, na kterých se shodl tým, jsme nastínili v prvním díle. Šlo o problémy, které jsme chtěli procesem code review řešit. A právě s tímto podkladem jsme si navrhli níže popsaný proces.

 

Základy

Jako důležitý odrazový můstek pro náš proces jsou naše vývojové standardy, ve kterých máme popsány různé aspekty vývoje, namátkou třeba:

• code style pro různé programovací jazyky (PHP, JS, CSS, ...)
• programátorský styl (používané návrhové vzory, techniky, ...)
• ideální architektonický návrh
• do & don'ts

V našem případě jde o repozitář s Markdown dokumenty. Každý z nich je vcelku krátký, stručný a většinou deklarativní, protože znalost „proč“ by měla být distribuovaná v týmu (vysvětlování zde má tendenci dokumenty neúměrně natahovat).

Aktualizace probíhají po týmové diskuzi, která je povětšinou vyvolána nějakou zkušeností... (Není nezajímavé, že v souvislosti s code review vychází velká část diskutovaných témat právě ze zkušeností.)

Proč je to důležité? Bez těchto základů bude mít code review tendenci degenerovat k manuálnímu whitebox testování, protože všechny ostatní aspekty (styl, architektura, ...) budou jen věcmi názoru, kolem nichž budou akorát vznikat přetlačované mezi vývojáři.

 

Proces

První otázkou je kdy, tedy v jakém momentu zahájit samotné code review. Máme na ni rovnou dvě odpovědi:

• Když je nějaký úkol hotov, tzn. zadání je splněno a práce je rebasovaná nad aktuální vývojovou větev v samostatné větvi. Cílem code review je v tomto případě práci důkladně zkontrolovat.
• Když má nějaký úkol vícero možných podstatně rozdílných řešení nebo když se zasahuje do architektury. Cílem code review je co nejdříve se týmově shodnout na vhodném („správném“) způsobu řešení před jeho implementací, protože případné předělávky jsou drahé a demotivující.

Vývojář, který code review potřebuje, přepne v systému pro projektové řízení úkol do stavu Code Review a zruší přiřazení úkolu sobě.

Druhá otázka je komu, tedy jakému člověku je code review přiděleno. Jde o otázku často diskutovanou, její univerzální řešení však podle nás tak úplně neexistuje. U nás to máme takto:

• Nepřiřadíme nikoho, tým sám si v rámci denních stand-upů či svého Slack kanálu úkoly ke code review rozebere.
• Proběhlo již předchozí code review (předběžné, vrácené, …), pak je přiřazeno přímo na předchozího reviewera.
• Pokud jde o specifickou doménu, ve které má výrazně větší přehled konkrétní člověk, případně jde o citlivou oblast, pak po předchozí domluvě přiřadíme danému člověku.
• A pokud code review leží delší dobu ladem a nikdo si ho nevšímá, pak je za jeho direktivní přiřazení zodpovědný projektový manažer týmu. Ale to se nám stává spíše výjimečně.

Třetí otázkou je, co přesně má dotyčný reviewer po převzetí úkolu do code review provést. U nás jsou to typicky následující úkony v pořadí dle uvážení reviewera:

• Rámcové prostudování zadání.
• Kontrola (úplnosti) splnění zadání.
• Kontrola pipeline – u nás aktuálně zahrnuje kontrolu code style, statickou analýzu a spouštění automatizovaných testů (jsou-li).
• Průchod kódu – zamyšlení se nad vhodností úpravy řešení, architektury, použitých konstrukcí, jejich hraničních případů...
• Build & spuštění & rámcové proklikání upravené funkčnosti – toto má spíš blíže k testu, avšak testy jsou zařazeny v procesu dále, a tak test zde, před zamergováním do vyvíjené větve, předchází rozbití buildu, které nás trápilo.

Po dokončení dá vědět výsledek zpátky vývojáři spolu s povolením k zamergování, typicky to u nás dopadá:

• Bez jakéhokoliv problému.
• Drobné problémy k opravě (styl, opomenutí, překlepy, pojmenování proměnných, ...).
• Problémy většího rázu vyžadující větší opravy.
• Problémy, kvůli kterým nešlo pokračovat v code review.

Úkol je pak znovu přiřazen vývojáři k opravě. Časem nám vznikly následující optimalizace:

• Pokud nebyl problém, často reviewer zamerguje sám.
• Pokud jsou jen drobné problémy, často reviewer rovnou provede opravy i merge, aby zbytečně nepinkal úkol tam a zpátky, zbytečně to zpožďuje vyřešení (je ale nutné vědět, že s tím jsou jednotliví vývojáři v pohodě).
• Pokud jsou jen drobné problémy a jsou vráceny, občas jsou vráceny i s povolením zamergovat si to sám, opět z toho důvodu, aby úkol zbytečně nepinkal tam a zpátky.

Pokud vývojář opravuje jednotlivé připomínky, pak úpravy během code review nesmějí být squashovány, výrazně to kazí možnost kontrolovat, zda bylo něco napraveno a jak. Squashování je však vyžadováno před mergem do vyvíjené větve.

Po zamergování je úkol nastaven na vývojářsky dokončený a posunut k testování.

 

Podpůrné nástroje

Bez podpůrných nástrojů by realizace každého code review byla extrémně náročná a zdlouhavá, my jsme došli k používání následujících:

• GitLab: Zde máme Git repozitáře, větve, merge requesty a nad kódem zde provádíme samotné code review a necháváme komentáře k jednotlivým řádkům kódu či obecně k celé větvi.
• GitLab CI: Umožňuje automatizovaně kontrolovat code style pomocí PHP Code Snifferu, staticky kontrolovat kód pomocí PHPStanu či například spouštět testy PHPUnitu. Zelená pipeline v GitLabu nám tak říká, že kód prošel našimi minimálními nároky, a code review tedy může začít. Zároveň nám tyto automatizované testy výrazně šetří čas a umožňují zkontrolovat další věci a soustředit se spíše na ty koncepční.

Ožehavou otázkou bylo i vypracování checklistu, v nějaké obecnosti se nám to i podařilo a je součástí našich vývojových standardů, avšak není příliš používán. Je hodně obecný a i při stručné implementaci je vcelku dlouhý (a může být prakticky nekonečný).

 

Nyní už víte, jak může vypadat konkrétní proces code review i jaké k němu lze použít nástroje. V příštím díle si povíme něco o tom, jak takový proces připravit a zavést.