Code review (kontrola kódu)

Příklady z praxe

1. 01

   Zachycená chyba v oprávnění

   Vývojář přidá kontrolu, zda uživatel smí upravit dokument. Recenzent si všimne, že podmínka používá přiřazení místo porovnání, takže by se oprávnění chovalo chybně. Po opravě a doplnění testu se změna sloučí bez bezpečnostního rizika.

   // Původní změna v pull requestu
   if (user.id = document.ownerId) {
     allowEdit();
   }
   
   // Po review
   if (user.id === document.ownerId) {
     allowEdit();
   }

2. 02

   Chybějící index před produkcí

   Tým přidává vyhledávání objednávek podle externího identifikátoru. Autor upraví aplikaci, ale v první verzi zapomene na index v databázi. Recenzent upozorní, že dotaz bude na rostoucí tabulce pomalý, a změna se doplní ještě před nasazením.

   ALTER TABLE orders ADD COLUMN external_id text;
   CREATE INDEX CONCURRENTLY orders_external_id_idx ON orders (external_id);

Časté omyly

MýtusCode review je hlavně hledání překlepů a stylu.

Ve skutečnostiCode review má největší hodnotu při hledání chyb v návrhu, bezpečnosti, správnosti a udržovatelnosti. Překlepy a formátování by měly řešit automatické nástroje.

MýtusReviewer má autorovi přepsat řešení podle sebe.

Ve skutečnostiRecenzent má vysvětlit problém, riziko nebo lepší směr, ne prosazovat osobní vkus bez důvodu. Autor změny zůstává odpovědný za výslednou úpravu.

MýtusKdyž máme CI, code review už není potřeba.

Ve skutečnostiCI spouští předem připravené kontroly, ale nerozumí záměru produktu ani kompromisům v návrhu. Code review doplňuje CI lidským posouzením kontextu.

Časté dotazy

Kolik lidí má Code review provádět?

Code review obvykle provádí vývojář, který zná danou část systému nebo typ změny. U běžných úprav často stačí jeden recenzent, u bezpečnostních zásahů, migrací dat nebo veřejného API může dávat smysl více lidí s různou specializací. Důležitější než počet je odpovědnost: recenzent musí mít čas změnu skutečně přečíst, ne jen formálně kliknout na schválení.

Musí Code review zastavit každou drobnou připomínku?

Code review nemusí blokovat sloučení kvůli každé stylistické drobnosti. Drobné připomínky je vhodné označit jako nezávazné, zatímco chyby ve správnosti, bezpečnosti, datové integritě nebo srozumitelnosti návrhu mají vyšší váhu. Tým by měl rozlišovat osobní preference od pravidel projektu. Formátování a podobné detaily je lepší vynucovat automaticky, ne ruční debatou pod každým pull requestem.

Proč Code review nenahrazuje automatické testy?

Code review nenahrazuje automatické testy, protože lidská kontrola a testování pokrývají jiné typy rizik. Testy ověřují konkrétní očekávané chování opakovatelně a rychle, zatímco recenzent hodnotí záměr, návrh, čitelnost a dopady na širší systém. Silný proces používá obojí: testy chrání před známými regresními scénáři, review pomáhá najít špatné předpoklady a neúplné řešení.

Patří do Code review kontrola formátování?

Code review má kontrolovat formátování hlavně nepřímo přes domluvená pravidla a automatické nástroje. Ruční komentáře k mezerám, pořadí importů nebo zalomení řádků obvykle plýtvají pozorností recenzenta. Pokud tým potřebuje jednotný styl, měl by používat formatter, lint nebo pre-commit kontrolu. Recenzent se pak může soustředit na chování programu, návrh a rizika změny.

Zdroje

1. OWASP Code Review Guide(otevře se v novém okně)OWASP Foundation
2. Git - Contributing to a Project(otevře se v novém okně)Git
3. Review pull requests(otevře se v novém okně)Microsoft Learn
4. Modern Code Review: A Case Study at Google(otevře se v novém okně)ACM, 2018

Související pojmy

• GitGit je distribuovaný systém pro správu verzí zdrojového kódu, který ukládá historii změn jako propojené commity a umožňuje práci s větvemi bez stálého připojení k serveru. Vývojář díky němu porovná verze, vrátí chyby, spojí práci více lidí a bezpečně publikuje změny do sdíleného repozitáře.
• Pull requestPull request je návrh změny v repozitáři, který žádá ostatní členy týmu o kontrolu a případné sloučení do cílové větve. Obvykle obsahuje diff, popis záměru, komentáře reviewerů, stav automatických testů a historii úprav, takže se rozhodnutí o změně neodehrává jen v chatu nebo e-mailu.
• CI/CDCI/CD je sada postupů a automatizací, které průběžně integrují změny kódu, ověřují je testy a připravují nebo provádějí nasazení aplikace. Cílem je zkrátit cestu od commitu k bezpečnému vydání, snížit ruční práci a rychle odhalit chyby v build procesu, konfiguraci nebo samotném kódu.
• Technický dluhTechnický dluh je stav, kdy software nese následky rychlejšího nebo kompromisního rozhodnutí, které zjednodušilo dodání, ale ztížilo budoucí změny. Dluh může být vědomý i nechtěný; projevuje se složitějším kódem, chybějícími testy, zastaralými knihovnami, křehkou architekturou nebo dražším provozem a postupně vytváří „úrok“ v podobě pomalejšího vývoje.
• RefactoringRefactoring je cílená úprava vnitřní struktury existujícího kódu bez změny jeho pozorovatelného chování. Vývojář při něm zjednodušuje návrh, pojmenování, duplicity nebo rozdělení odpovědností tak, aby se kód snáz četl, testoval a měnil, aniž by uživatel aplikace poznal rozdíl ve funkcích.
• LinterLinter je nástroj statické analýzy, který kontroluje zdrojový kód bez jeho spuštění a upozorňuje na porušení pravidel, podezřelé konstrukce nebo nekonzistentní styl. Pomáhá zachytit chyby dřív než při testování či code review a často se spouští v editoru, před commitem nebo v CI pipeline.
• Pair programmingPair programming je způsob vývoje softwaru, při kterém dva lidé současně pracují na stejném úkolu, obvykle u jedné obrazovky nebo ve sdíleném editoru. Jeden píše kód jako driver, druhý průběžně přemýšlí jako navigator, kontroluje směr, hledá rizika a navrhuje další kroky.
• Merge conflictMerge conflict je stav ve verzovacím systému, kdy Git nedokáže bezpečně spojit dvě změny do jednoho výsledku, protože zasahují do stejné části souboru nebo do související struktury projektu. Vývojář musí konflikt ručně vyřešit, vybrat správný obsah, otestovat výsledek a dokončit merge, rebase nebo cherry-pick.