21. října 2008

jOpenSpace 2008 - Metodiky vývoje - jak děláme review kódu

Dneska vám předkládám poslední reakci na první dávku audií z jOpenSpace, po ORM a dokumentaci mě oslovila diskuse na téma metodiky vývoje. Poslední 2 zaznamenaná témata mě k nějakým poznámkám nevyburcovala.

Co se metodiky vývoje v podstatě mě velmi mile překvapilo, že se již naplno prosazují iterativní a agilní způsoby vývoje, které začínají odsouvat nevhodný vodopád. To je velmi potěšující. Je až nepochopitelně překvapující, že se vodopád udržel tak dlouho (mimo jiné podle knihy Craiga Larmana Agile and Iterative Development: A Manager's Guide je zjištění jeho nefunkčnosti hodně staré).

Poslední věc z diskuse je taky všude stejná. Zapojit zákazníka do vývoje je velmi težké, ale pokud se to podaří, je to super věc. My se především staráme o jednu už né-malou aplikaci, kterou poskytujeme jako službu. Je-li zákazník interní (vymyslíme si nějakou funkcionalitu, či vyvíjíme něco dovnitř) pak je to super, je-li zákazník externí, pak je odezva většinou pomalá.

Co ovšem vzbudilo mou chuť napsat tento příspěvek, je poměrně složitý způsob reviewováví kódu, které bylo nastíněno. Protože s naším systémem jsem poměrně hodně spokojen a mám pocit, že je na úrovni, rád se s ním s vámi podělím. Review jsem od začátku zamýšlel tak, aby každý kód, který vytvoříme byl reviewován. Z počátku jsme kontrolovali příchozí změny z CVSka. Pak nás začlo být v teamu víc než 2 a tento postup přestal fungovat.

Zhruba v tu dobu jsme nasadili systém JIRA pro evidenci požadavků. Tím se nám nabídla možnost evidovat jakoukoliv práci na projektu jako požadavek, který definuje, kdo jej bude implementovat a zároveň kdo jej bude reviewovat. Požadavek nemůže být zavřen, pokud nebylo provedeno review. Jednoznačnou výhodou je přehled toho, co se na projektu děje. Dále review provádíme průběžně a nikoliv až na konci iterace (přecijenom dělání review není moc populární a zajímavé), navíc je review skutečně provedeno na každý změněný řádek a to považuji za podstatné (víc očí víc vidí a rozumí-li kódu dva mozky, pak jim pravděpodobně bude rozumět i třetí).

Jako vývojový nástroj používáme Eclipse a proto jsem hledal podporu review kódu integrovatelnou do Eclipse. Jediné co jsem našel byl Jupiter (koukám, že se na něm usilovně pracuje, tak třeba se zlepší). Dlouho jsme jej používali a zjistili jsme, že má řadu nevýhod a proto jsem hledal náhradu. Koukal jsem na Crucible, ale nelíbilo se mi, že by evidence review byla mimo již existující infrastrukturu (podobně jako v případě Jupiteru).

Jako zatím top volba se ukazuje integrovat systém JIRA s naším subversion serverem (pomocí subversion pluginu) tak, aby u každého požadavku zobrazoval change sety, které se jej týkají (k tomu stačí, aby komentář commitu do subversion obsahoval kód požadavku). Následně jsme potřebovali nástroj na zobrazení obsahu subversion a volba padla na Fisheye, protože je sofistikovanější než ViewVC.

A jak samotné review probíhá? Je-li požadavek vyřešen, pak je nutno provést review. Podívám se na požadavek a vidím jaké soubory, byly v souvislosti s jeho řešením commitovány. Přímo ze systému JIRA je možno se prokliknout do Fisheye, kde potřebuji další klik na zobrazení změn souboru s požadavkem souvisejících. Je-li vše v pořádku, pak stačí požadavek označit jako reviewovaný a pak už jej může vedoucí projektu zavřít.

Není-li vše v pořádku, pak přijde na řadu diskuse. Tu nijak nedokomuntujeme a je věcí programátorů, jak ji zrealizují. Pokud se ukáže, že je nutné provést změnu kódu (překvapivě se to děje docela často), pak je v systému JIRA vytvořen další požadavek na změnu (nepřehledný kód, nějaká chyba v kódu, chybějící test atd.). Tento požadavek je normálně řešen a je i reviewován jako každý jiný. Commit do subversion obsahuje kódy obou požadavků, aby byl v systému JIRA spojen s oběma (jak s původním, tak s review).

A jaký má systém výhody a nevýhody? Řadu už jsem jich zmínil, ovšem další výhoda tkví v přesné evidenci toho co se děje, v jakém stavu je který požadavek a co se v souvislosti s jeho plněním měnilo a jak. To je neocenitelné. A nevýhody? Pokud je požadavek hodně rozsáhlý, pak je složitější se v souborech vyznat. Je-li to umocněno více change sety (tj. více commity), je to skutečně náročné.

A uplným závěrem motivační větu: Review kódu skutečně zvyšuje jeho kvalitu a napomáhá dřívějšímu odhalení chyb. Navíc se programátoři učí číst kód druhých, orientovat se v něm, komunikovat o něm a lépe poznávají systém, který programují.

2 komentáře:

RB řekl(a)...

Přímo ze systému JIRA je možno se prokliknout do Fisheye, kde potřebuji další klik na zobrazení změn souboru s požadavkem souvisejících. Je-li vše v pořádku, pak stačí požadavek označit jako reviewovaný a pak už jej může vedoucí projektu zavřít.


Nediskutuji nyní až tak o nástrojích, spíše by mě zajímal ten proces. Píšete, že pokud je všechno v pořádku, což jsem si přeložil, jakože vedoucí projektu nazná, že je vše v pořádku, je požadavek uzavřen. Moje otázka je, zda je možné, aby vedoucí projektu ze své pozice byl tou centrální autoritou, která rozhodne, že je vše v pořádku. Má na to (v realitě) takový vedoucí "background"?

Jira řekl(a)...

Tím je vše v pořádku, jsem myslel, že programátor konající review nezjistil žádné problémy v kódu, které sám neopravil (především překlepy v komentářích) a nebo nevytvořil review issue.

Fakticky je možné, aby review prováděl vždy vedoucí projektu a pak by měl background na to vědět, že je vše v pořádku, ale to by nedělal skoro nic jiného. Proto review dělají různí programátoři. Jako vedoucí projektu si namátkou něco prohlédnu sám a každopádně si to proklikám, abych věděl jak to funguje.

Takže požadavek zavřu pokud jsou všechny review uspokojivě vyřešena a nebo žádná nevznikla a důvěřuji kolegům, že pracují pečlivě. Ovšem je na mě, zda se na něco dobrovolně podívám nebo ne.

Navíc je možné podle toho kdo požadavek řeší určit kdo dělá review (i více lidí) a tím zařídit že senior programátor je kontrolován juniorem (junior se učí) a naopak (junior je pod dohledem) a nebo jedná-li se o kritické části systému, pak na ně hledí dvoje oči seniourů.