Úspech projektu sa dá rátať na odpracované hodiny alebo manažérsky – na hodnotu dodanú zákazníkovi. Ak sa zákazníkovi splnia obchodné ciele, je to úspech.
V Bart.sk sa orientujeme na dlhodobé projekty a dlhodobé ciele zákazníkov. Dokončiť a odovzdať krátky projekt je pomerne ľahké. Trvale a konzistentne dodávať kvalitný produkt po dlhý čas je niečo, čo si vyžaduje iný pohľad na to, ako sa píše a maintainuje kód.
Code review je jeden z nástrojov na zvýšenie kvality vášho tímu a kódu samotného. V podstate ide o proces, v rámci ktorého jeden alebo viac ľudí kontroluje váš kód pred tým, než je začlenený do projektu.
S „code review“ sa môžete stretnúť ako s merge requestom (MR) na GitLabe alebo s pull requestom (PR) na GitHube. Ďalej používam názov MR, pretože fičíme na GitLabe, jedná sa však o jedno a to isté.
Prečo je code review dôležité?
Každý vnímame svoj kód ako to najlepšie, čo z nás v ten deň vypadlo. Pravdou ale je, že ak nad riešením konkrétneho problému strávime veľa času, naše vnímanie problematiky sa môže scvrknúť a uniknú nám tak širšie súvislosti. Napríklad, že naša funkcia alebo komponent vyrieši jeden konkrétny problém, ale zabudneme ošetriť niektoré vstupy, pre ktoré nám nenapadne use-case.
Takisto všetky premenné, názvy funkcií a algoritmy sú pre nás úplne jasné. Obyčajne ale pomôže, ak sa na kód pozrie niekto iný a zhodnotí, či sa dobre číta. Väčšinu času aj tak kód programátori iba čítajú.
U nás si revieweri často kód aj sami vyskúšajú. Pokiaľ napríklad ide o MR pre nejakú časť UI, môžu takto prísť na vylepšenia, a ak skúsia UI použiť tak, ako programátor nezamýšľal, nájdu chyby.
Ak do MR zapojíte aj QA (Quality Assurance), viete dokonca prísť na chyby už v tejto fáze, čo vám ušetrí čas neskôr.
Hlavný prínos MR vidím v zvyšovaní kvality kódu a znižovaniu počtu bugov, avšak nemenej dôležité je, že všetci zúčastnení sa pomocou MR učia. Učia sa používať technológiu, spoznávajú projekt.
Z mojej skúsenosti vám môžem povedať, že každý je omylný. Nezáleží na seniorite. Stále sa človek môže naučiť niečo nové.
Code review môže slúžiť aj na to, že si urobíte interný workshop. Napr. Stano nám nedávno urobil workshop kde nám na relatívne veľkom MR vysvetľoval ako migroval kód na najnovší Angular, commit za commitom.
Ako vyzerá dobrý MR
Obyčajne platí, že MR by mal mať výborný názov a výborný popis preto, aby dokonca aj nový kolega v tíme bol schopný urobiť code review.
Ak ide o fix bugu, popíšte ako sa bug prejavuje, na ako environmente, na akom účte a aké sú presné kroky k zreplikovaniu. Pri novej feature je výborné, ak je v popise napr. dizajn, špecifikácia, požiadavky a iné upresňujúce informácie. Vtedy reviewer vie získať dostatočný prehľad o tom, čo ide kontrolovať.
Dajte si pozor aj na rozsah. Osobne som radšej, ak ako reviewer mám desať MR s jednou zmenou ako jeden MR s desiatimi zmenami. Trochu potom strácam focus. Inými slovami:
10 lines of code = 10 issues.
500 lines of code = „looks fine.“Každý code review
Dobrý MR nie len opravuje bug alebo pridáva feature, ale vylepšuje, čo už na projekte je.
Podľa môjho názoru je ale aj tak najlepší MR taký, ktorý z projektu uberá, namiesto toho, aby pridával. Ak popri tom aj opravíte bug alebo pridáte fíčuru, zaslúžite si jednu slivkovú guľu na obed navyše.
Ako dávať feedback a nebyť za blbca
Mnoho krát som sa stretol s názorom juniornejších programátorov, že majú dokonca radi, ak sa ich kód skritizuje. To je z môjho pohľadu výborný pohľad na vec kedy „hlad“ učiť sa je väčší ako ego :) Pozor ale na to, aby ste to s kritikou neprepálili.
Najdôležitejšia zásada pri dávani feedbacku je – nebyť blbec. Obyčajne stačí, ak vaše ego necháte doma a spomeniete si, aké skvosty ste kódili kedysi vy. Každý sa učí.
Na začiatok code review skúste nasať čo najviac informácii o danom kóde. Hlavne teda prečo vznikol a čo má riešiť.
Keď začínate hodnotiť a uvidíte, že váš kolega podľa vás napísal úplnu blbosť, dajte si pauzu a nepíšte review hneď. Ak vám to ani po pár minútach nedá, a už ste preverili okolnosti onoho kódu, píšte v otázkach alebo aspoň neprikazujte. Ponúkajte vylepšenia, navrhujte. „Myslíš, že by to šlo lepšie, ak by si použil …?“, „Čo keby si vyskúšal tento … algoritmus“, „Myslím si, že toto by sa dalo vyriešiť takto …“. Za každých okolností sa snažte byť objektívny a emócie sa pokúšajte držať na uzde.
Ak komentár už dáte, trvajte na uspokojujúcej odpovedi. Odpoveďou by nikdy nemalo byť, že „takto to tu proste robíme“ alebo „skopčil som to zo stack overflow“, príp. „fixneme to, keď bude čas“ (spoiler alert: nefixnete).
Pre programátorov je komunikácia s inými ľudskými tvormi obyčajne dôvod, prečo radšej komunikujú s počítačmi. Pri code review ale musíme tento zabudnutý soft skill obnoviť (alebo nájsť). Text nenesie veľa emócie, preto používajte emotikony a prekomunikujte svoju kritiku. Nebojte sa kolegu pochváliť, dobre to padne.
Keď vám niekto urobí code review
Komentáre neberte osobne.
Ako programátori sme svojím spôsobom umelci a keď niečo vytvoríme, sme na to hrdí a nie vždy je jednoduché prijať kritiku. Najmä potom, čo sme na niečom pracovali niekoľko hodín alebo dní. Beztak stáva sa, že tam máme chyby alebo niekto má lepší nápad alebo lepší algoritmus.
Zostáva nám zobrať to s pokorou a naučiť sa niečo nové. Kritiku vitajte s otvorenou náručou. Feedback, aj kritický, je pre dobro projektu, vás aj vášho zákazníka.
To avoid criticism, do nothing, say nothing, and be nothing.
Elbert Hubbard
Ako dlho by mal trvať code review
Toto je častá otázka aj v skúsenejších tímoch. Samozrejme, ak čakáte, že dostanete nejaké konkrétne číslo, čakáte márne.
Skúste sa ale zamyslieť, čo code review predchádza.
Obyčajne sa píšu požiadavky, robí sa dizajn, píšu sa funkčné špecifikácie, technická analýza a nakoniec sa programuje.
Programátor musí pochopiť, prečo niečo ide kódiť, aké sú tam súvislosti, aké obmedzenia, závislosti a požiadavky. Obyčajne niečo nakódi, potom to prepíše, nájde lepšie riešenie, opäť prepíše a nakoniec svoj kód, na ktorom pracoval dva dni, odovzdáva na code review.
Spýtajte sa sami seba, koľko potrebujete času na to, aby ste pochopili riešenú problematiku a skontrolovali, či kód dodá sľúbenú hodnotu? Niekedy si potrebujete kód aj spustiť, pretestovať ho sami a spísať hodnotný feedback. Neviem, koľko času potrebujete, ale pravdepodobne to bude signifikantná časť tých dvoch programátorských dní.
Nestrácajte čas
Príkladov ako strácať čas s code review je neúrekom. Môžete napríklad hľadať chýbajúce bodkočiarky alebo sa týždne hádať, či budete používať taby alebo medzery, prípadne to či dáte zátvorku na nový riadok, alebo na ten istý.
Skúste čo najviac vecí doladiť strojovo, ešte predtým, ako reviewer prvýkrát uvidí kód. Snažte sa mať zautomatizované všetko, čo viete. Dávajte ale pozor, aby sa vám to nevymklo z rúk.
Zdroj: https://xkcd.com/1319/
U nás sme si to zariadili tak, že po každom push evente do merge requestu (rozumej do vetvy, z ktorej je vytvorený MR) sa pustí pipeline (viac o GitLab pipelineách tu), v ktorej sa skontroluje:
- či je kód sformátovaný podľa dohodnutého code style,
- či prejde statická analýza (set pravidiel pre best practices),
- či prejdú unit testy,
- či sa dá projekt buildnúť.
Takýto „čistý“ MR potom u nás kontrolujú dvaja ľudia, ktorí sa teraz naplno môžu venovať tomu, aby hľadali chyby v logike, potencionálne bugy, bottlenecky, nekonzistencie alebo aby aspoň chvíľu lamentovali nad zlým pomenovaním premenných.
Ak ide o MR nižšej dôležitosti, stačí jeden reviewer.
Bonus: prepojenie s issue trackerom
Prepojenie so svojím obľúbeným issue trackerom si naprogramujete za pár hodín a ušetrí to stovky hodín programátorom.
Naše automatizované prepojenie s JIRA zabezpečuje, že sa do popisu MR pridá link na konkrétny JIRA issue, v ktorom je popísané „čo“ a „prečo“. Programátori nemusia vypisovať popisy MR ručne, všetky detaily, či už o bugu alebo feature, sú v JIRA issue. Bonusom je, že issues v JIRA sa automaticky presúvajú na základe stavu MR. Programátor tak nestráca čas „byrokraciou“.
Toto prepojenie nám dovoľuje vytvárať aj tabuľky (nižšie), ktoré si ukazujeme na denných standupoch – zobrazujú, kto má priradených koľko MR a v akej verzii. Vďaka nim máme rýchly prehľad o tom, kto je ako vyťažený a ktoré MR je potrebné uprednostniť.
