Proč code review nezachrání architekturu, pokud se ptáte jen na řádky

Pull request projde třemi schváleními za dvacet minut. Někdo zkontroloval pojmenování, někdo si všiml chybějícího testu, někdo ocenil, že diff je přehledný. Všechno v pořádku, merge. Za půl roku je přesně tenhle PR součástí důvodu, proč už se systém nedá rozumně měnit. Review přitom neselhalo — udělalo přesně to, k čemu je. Jeho úkol jen nebyl architektura.

01Co code review umí dobře

Začněme tím, co code review opravdu funguje. Rituál review nad diffem je výborný na úrovni řádků: chytá chyby v logice, špatné pojmenování, odchylky od stylu, chybějící testy, bezpečnostní zápachy v konkrétních změněných řádcích. To je reálná hodnota a žádný tým by se jí neměl vzdávat.

Problém není v tom, že by code review bylo špatné. Problém je v tom, že od něj týmy očekávají i to, že ohlídá architekturu — a to strukturálně nedokáže. Není to otázka pečlivosti recenzenta ani délky review. Je to vlastnost samotného nástroje: code review se dívá na změnu, ne na tvar systému.

02Proč diff architekturu neukáže

Diff je z principu lokální. Ukazuje jednu změnu na jednom místě: tyhle řádky přibyly, tyhle zmizely, tady se něco přepsalo. Architektura je ale něco jiného — je to vztah mezi soubory: graf závislostí, kde leží hranice modulů a služeb, kterým směrem závislosti míří.

Pull request ukáže jednu novou hranu v tom grafu. Neukáže, že ta hrana právě uzavřela cyklus závislostí. Neukáže, že modul teď sahá přes hranici, kterou neměl překračovat. Neukáže, že stejný typ volání tady přibývá popáté. Recenzent vidí strom — ten konkrétní změněný soubor — a nevidí les, tedy strukturu, do které ten soubor zapadá.

To není nedbalost. Je to limit informace, kterou diff vůbec nese. Aby recenzent viděl architektonický dopad, musel by si v hlavě nebo na papíře držet celý graf závislostí systému a promítat do něj každý PR. To se v rámci dvacetiminutového schválení nestane.

Někdy ji ale chytne i tak. Pozorný recenzent — typicky senior, který nosí v hlavě mapu systému, ale občas i kdokoli, kdo dotčený kus kódu důvěrně zná — dokáže u PR zarazit změnu, jež je na úrovni řádků v pořádku, ale „patří jinam“. Kus architektonického review tím odvede přímo v pull requestu. Není to ale nástroj, který funguje; je to člověk, který supluje, co nástroj neumí. Stojí to na paměti a kapacitě jednoho člověka, nepřenese se to na zbytek týmu a je to přesně ten „jeden strážce“, před kterým článek dál varuje. Code review architekturu občas zachytí — jen se na tom nedá postavit proces. A o proces tady jde.

03Otázky, na které review neodpovídá

Nejlíp je to vidět na otázkách, ke kterým se review zaměřené na řádky nikdy nedostane. Kdo teď na kom závisí? Překročila tahle změna hranici modulu nebo služby? Je tohle vůbec správný modul pro tuhle logiku? Neudělali jsme právě něco, co bude později drahé měnit? Dá se tohle rozhodnutí ještě vzít zpět?

Ani jedna z těch otázek není vidět v textu diffu. Diff je zodpovědět neumí — a přitom všechny jsou architektonické. Recenzent, který poctivě prošel každý řádek a nic mu neuniklo, může schválit PR, který je na úrovni řádků dokonalý a na úrovni struktury katastrofa. Diff mu k tomu rozdílu nedá jediné vodítko.

04Architektonické review je jiná aktivita

Z toho plyne potřeba přerámování. Architektonické review není „code review, ale pečlivější“. Je to jiná aktivita — s jinou kadencí, jinými otázkami a někdy i jinými lidmi. Samo o sobě to není nový objev: že struktura systému je něco jiného než jeho řádky, ví obor přinejmenším od Conwaye a literatura kolem mikroslužeb to rozebírá desítky let. Nové není to zjištění — nový má být návyk: udělat z toho rozlišení pojmenovaný, naplánovaný krok, ne tichý předpoklad.

Review řádků probíhá u každého PR; je rychlé, časté, lokální. Architektonické review probíhá u každé významné změny — a těch je výrazně méně. Má smysl se nad nimi zastavit déle, podívat se na graf, zeptat se na hranice. Když obě aktivity natlačíte do jediného dvacetiminutového schválení, ta architektonická polovina se zaručeně přeskočí. Není na ni čas a není to ani její formát. A část architektonického review nepatří ani do PR — patří před něj. RFC (krátký návrhový dokument k oponentuře, než se začne stavět) nebo nákres hranic dřív, než vznikne první commit, řeší cenu vratnosti: rozhodnutí v návrhu se ještě dá levně přepsat, rozhodnutí zakopané v diffu už stojí migraci.

05Praktický checklist architektonického review

Architektonické review nemusí být těžký proces. Stačí krátký, konkrétní checklist, který se projde u významné změny:

• Směr závislosti — přidává tahle změna závislost? A míří správným směrem, nebo vytváří hranu, která by tam být neměla?
• Překročení hranice — sahá změna přes hranici modulu nebo služby? A je to záměr, nebo náhoda?
• Vlastnictví — patří dotčený kód týmu, který ho mění? Pokud ne, vzniká vazba přes hranici týmů — a právě tohle popisuje článek Architektura je tvar týmu.
• Vratnost — pokud je tohle rozhodnutí špatně, jak drahé bude vzít ho zpět? Odpoledne, nebo migrace?
• Skryté rozhodnutí — není v tomhle PR zakopané architektonické rozhodnutí, které mělo proběhnout jako ADR nebo RFC dřív, než se psal kód? Pokud ano, schválit ho jako diff znamená rozhodnout zpětně — a draho.
• Kumulativní zátěž — tohle je N-tá podobná výjimka. Je ten vzor pořád ještě přijatelný, nebo se z výjimky stalo pravidlo?

Checklist není o tom najít „špatný kód“. Je o tom položit otázky, které diff neklade — a udělat to vědomě, ne doufat, že to někoho napadne.

06Co z checklistu zvládne stroj

Část toho checklistu nemusí řešit člověk. Směr závislosti, překročení hranice modulu, cyklické závislosti — to všechno jsou vlastnosti grafu importů a graf umí číst stroj. ArchUnit hlídá pravidla závislostí přímo v testech, dependency-cruiser dělá totéž nad grafem importů v JavaScriptu a TypeScriptu, statická analýza obecně zvládne „tahle hrana sem nepatří“ a Structurizr nebo podobný nástroj ten graf zviditelní, aby se o něm vůbec dalo mluvit.

Rozdíl je zásadní. Když máte v CI test, který padá ve chvíli, kdy někdo zavede závislost přes hranici, kterou nemá, ta otázka se v PR vůbec neklade — odpoví na ni stroj dřív, než se na pull request kdokoli podívá. Tři z šesti položek výše — směr závislosti, překročení hranice a cykly jako jejich podmnožina — se takhle dají z velké části zmechanizovat.

Neznamená to, že architektonické review není potřeba. Znamená to, že se nemá utápět na věcech, které detekuje nástroj. Když mechanizovatelnou polovinu hlídá CI, lidský čas se uvolní na otázky, na které stroj odpověď nemá: vratnost, vlastnictví a kumulace. Ta poslední — kdy se z opakované výjimky stane pravidlo — si zaslouží vlastní pohled.

07Katastrofa po kapkách

Z těch šesti otázek je jedna těžší než ostatní: kumulativní zátěž. Architektonická katastrofa skoro nikdy nepřijde jedním špatným PR. Přijde sečtením — desítek rozumných ústupků, z nichž každý jednotlivě dával smysl. Pátá výjimka ze stejného pravidla není pětkrát horší než první; je to okamžik, kdy se z výjimky stalo pravidlo a nikdo to neoznámil.

A tady je past, kterou samotný checklist nevyřeší: jak tu pátou výjimku vůbec poznáte? Každý recenzent vidí jen svůj PR. Často je to pokaždé někdo jiný. Pátý diff nenese informaci, že čtyři stejné už prošly — a člověk, který ho schvaluje, o těch čtyřech nemusí vědět. Kumulace je z principu neviditelná v každém jednotlivém PR. Právě to je nejsilnější důvod, proč ji review po jednotlivých PR nikdy nezachytí: ne proto, že jsou recenzenti nepozorní, ale proto, že potřebná informace v jednom diffu prostě není.

Vidět ji jde jen odjinud. Stroj umí počítat — „tenhle vzor je v kódu posedmnácté“ je dotaz, na který statická analýza odpoví. Zaznamenané výjimky pomáhají taky: když každý vědomý ústupek skončí jako ADR: architektonická paměť týmu, je počet najednou na jednom místě a viditelný. A periodický pohled přes víc PR najednou — krátké sezení jednou za sprint, kde se neprochází kód, ale graf změn a opakující se vzory za dané období — je místo, kde si toho někdo konečně všimne. Není to revize jednotlivých rozhodnutí; je to ohlédnutí za tím, kam se za poslední dva týdny posunulo těžiště systému. Stroj i záznam vám řeknou, že výjimka je pátá. Jestli je ten vzor pořád ještě přijatelný, nebo se má zastavit, je rozhodnutí — a to zůstává na člověku.

08Kdy review eskalovat z řádků na strukturu

Většina PR architektonické review nepotřebuje — opravuje chybu, doplňuje test, mění text. Důležité je rozpoznat těch pár, které ho potřebují. K tomu pomáhají jednoduchá spouštěcí pravidla.

Když PR přidává závislost mezi moduly, zavádí novou hranici, mění datový model nebo je N-tým opakováním stejného vzoru — označte ho k architektonickému review místo toho, aby se jen schválily řádky. Stačí štítek, položka v checklistu nebo brána „needs arch review“. Přidat to stojí pár minut a udělá to z eskalace explicitní krok, ne naději, že si toho někdo všimne.

U některých změn ale eskalace neznamená „přidej druhé review k tomuhle PR“ — znamená „tenhle PR teď nezavírej“. Pokud spouštěč skočí na nové hranici nebo změně datového modelu, je často levnější PR pozastavit, napsat RFC a teprve pak se vrátit ke kódu. Diff pak diskutuje hotové rozhodnutí, ne ho dělá. Rozeznat tyhle dva módy — „doplň review“ vs. „zastav PR“ — je samostatná dovednost a stojí za to ji v týmu pojmenovat.

09Kdo a kdy

Zbývá otázka, kdo architektonické review dělá a kdy. A tady je potřeba být upřímný: to review není zadarmo. Přidává do cesty změny latenci. Vytváří další místo, kde se rozhodnutí může zaseknout. A v týmu bez silných seniorů se snadno zvrhne přesně v toho „jednoho strážce, přes kterého teče všechno“ — úzké hrdlo, u kterého se tým naučí čekat místo přemýšlet. To je první instinkt a je to past.

Lepší jsou lehčí varianty. Rotující role „architektonického recenzenta“, která každý sprint připadne někomu jinému. Stálá dvojice, která drží sprintové ohlédnutí za hranicemi — jeden přináší hloubku, druhý kontinuitu. Požadavek na ADR u označených změn. Žádná z nich ale není zadarmo a žádná nefunguje všude stejně — rotace zvlášť ne. Ne každý v týmu má hloubku, aby ji táhl rovnocenně; rotovat review mezi lidmi s nestejnou zkušeností znamená, že některý sprint bude mělčí. To není důvod ji zahodit, ale důvod nepředstírat, že je tím věc vyřešená.

Princip bych proto neformuloval jako „udělejte architektonické review explicitní“ — to zní jako další povinná schůzka. Spíš: udělejte explicitní moment, kdy se klade jiná otázka než u řádků. Jestli ten moment bude schůzka, ADR, štítek na PR nebo padající CI test, ať se přizpůsobí týmu a tomu, kolik seniority v něm reálně je. V týmu bez seniority pojmenování momentu samo expertízu nevykouzlí — tam je upřímnější přiznat, že část architektury bude na začátku muset přijít zvenčí (mentor, externí review, cizí ADR jako vzor), než předstírat, že rotace problém vyřeší. Důležité je, že ten moment existuje, je pojmenovaný — a že odpovídá tomu, kdo v týmu skutečně je.

10Závěr: dva nástroje, dvě otázky

Code review nezachrání vaši architekturu, protože k tomu nikdy nebylo stavěné. Je to nástroj na úroveň řádků a tu úroveň zvládá dobře. Architektura potřebuje něco jiného — jinou otázku, jinou kadenci, část práce klidně automatizovanou, někdy jiné lidi.

Neházejte proto code review přes palubu. Nechte si ho na řádky, kde je užitečné. Mechanizovatelné hranice dejte do CI. A pro zbytek — vratnost, kumulaci, vlastnictví — si vyhraďte explicitní moment, kdy se ptáte na strukturu, ne na řádky. Bez něj budete dál schvalovat systém do kouta — jeden rozumný PR po druhém, každý z nich sám o sobě v pořádku.

Code review se ptá: je tahle změna dobře napsaná? Architektura potřebuje jinou otázku — a klást ji nejlíp dřív, než se začne psát kód: je tahle změna vůbec na správném místě?