2016. december 4., vasárnap

final code-review-review

Vannak érvek a codereview mellett és ellene is. Kellett hozzá néhány év türelem, had gyűljenek az élmények. Ragyogó elméletek kontra szőrös valóság. Legyenek akkor elöbb a pro, mert az egyszerűbb, és sajnos sokkal rövidebb is.

Pro - ami működött


Egyik régi munkaadómnál a külső beszállítók gyakorlatilag review nélkül, és a management nyomására nem elég ritkán tesztelés nélkül is élesbe állították a rendszereiket. Mindenki boldog volt, amíg el nem szállt. És akkor jött a körkérdés: "Ért itt valaki groovy-hoz?" mire a legtöbben: "Mihez?"
Élesben fut egy rendszer, azt se tudtuk hogy mit csinál és ki használja, de elhasalt és fel kell támasztani.

Ugyanitt önként és meghívásos alapon elkezdtünk egymás között egy code-review szerűséget. Tea vagy narancslé, két szék, egy képernyő, együtt átnéztük a szoftver egy részét. Az ötlet az volt, hogy a review-er egyúttal backup ember is lehet, ha az eredeti fejlesztő nem elérhető, mert mondjuk elütötte egy autó. Például ez meg is történt velem.
A review során a review-erek inkáb csak ötleteket adtak, nem kötelező jellegű utasításokat. Jópár nagyon jó és hasznos ötletet kaptam és ezeket a review-ket úgy tünt mindkét oldalon pozitívan értékeltük. Mindkét fél ott ült, mindenki csak erre figyelt, elég gyorsan ment. A pár-hetente pár óra aligha lassította a fejlesztést, ugyanakkor viszont arra nem volt jó hogy konkrét hibát találjon.

Kontra


A szorosabb review process ötlete főleg, de nem kizárólag az open source projektek jellemzője. Mondjuk egy open source projekten tényleg át kell nézni az akárkiktől érkező patcheket, de ezzel sok probléma akadt:


Elösször is léteznie kellene egy alap kritérium listának, ami alapján elindul az ember, amolyan checklist. Ilyesmiket, mint kódformázással kapcsolatos szabályok. Ilyen többnyire nincs és helyette olyanokat szoktak mondani, mint "common sense", "well known traditions". Ez nem működik, ami az egyik kultúrában értelmes, az a másikban nem. Pl ami a spring-ben normális, az Java EE-ben nem az.
A helyzetet súlyosbítja, ha több reviewer is lehet, ugyanis többnyire ők sem értenek egymással, ami átmegy az egyiken fennakad a másiknál és fordítva.

Aztán a másik dolog ami a code review igéretei közűl megmaradt igéretnek az a párbeszéd. Egy webappon keresztül akarunk beszélgetni? Ne tessék viccelni, már a shared desktop + skype is elég szűkös néha, mert nincs hova rajzolni, lag-el a vonal, nem értjük elég jól egymást, esetleg a nálam már hajnalodik, a másik fél viszont még nem ebédelt.
Itt egy kicsit a kultúrális különbségek bejátszottak. Például sok izraelli munkatársam még mindig aktív katonai szolgáltaban állt, ők a command chain-hez voltak hozzászokva, az ő napi megszokásuk az volt, hogy a besztottak végrehajtják a parancsot. Abból lesz ám fasza dolog :)
Más kultúrákban is van így, például sok indiai is ha egyszer mondott valamit, akkor nagyon nehezen, vagy egyáltalán sehogy se tud kihátrálni. Persze ismerek kivételeket köztük is, de ez a rugalmatlanság amerikaiaknál és európaiaknál ritkábban fordul elő.

Harmadik beteljesítetlen igéret a kevesebb bug a kódban. A probléma talán onnan jön, hogy egy webappon keresztül nézegetik a reviewerek a kódot. Az hogy letöltsék és ki is próbálják, az opcionális, és mivel sok időt vesz igénybe, úgy látom többnyire nem is történik meg. Ezt a legtöbben be is vallották és azt mondták, a patch fejlesztőjének a felelőssége a tesztelés. Ebben nem értek egyet, teszt nélkül szerintem a review teljesen irreleváns.
Egy esetben pl 5 hónapig pöckölgettünk egymásnak patcheket, a végén a management nyomására lett vége a sztorinak. Bár egy délután alatt bőven le lehetett volna tesztelni a kódot, sajnos ez alatt az idő alatt én voltam az egyetlen aki kipróbálta.

A negyedik elmaradt igéret a tisztább kód. Bár a code review elvileg kivállóan betartatná a konvenciókat, a valóságban gyakran ez sem így történt. A már meglévő kód takarítása gyakorlatilag megvalósíthatatlanná vállt. Nem maradt rá idő. Amikor mégis beküldessz egy kis patchet, akkor a review gudelines hiánya miatti félreértések következnek: vedd még mást is hozzá illetve már így is túl sok, várj még a patch-csel illetve elavult és légyszi rebaseld.


Az ötödik probléma a review-val a határidő. Sajnos a reviewerek a gyakorlatban teljesen leszarták a határidőket. Ez már management hiba, de meg is tehették, mert rajtuk senki sem kérte számon. Gyakran hetekig vagy akár hónapokig is eltartott egy review, közben nem történik semmi. Ez két további problémát vet fel:
  • Nagyon gyakori task-switching. Ebben a gépek a nyerők, az embernek sok időbe tellik és a párhuzamos taszkok számával exponenciálisan nő a valószinűsége annak, hogy elcseszi. Csinálj egy dolgot, csináld addig, kész nem lesz!
  • Ha nem tudok igéretet kapni a reviewerektől a határidőkre, akkor hogyan tudnék én igéretet adni határidőkre? Ez a legsúlyosabb probléma a code review-vel a hétköznapi életben.

Szóval...

A code review mögötti ötlet érthető, csak a gyakorlati megvalósítása elött van egy pár akadály, amit a projekt vezetők gyakran figyelmen kívül hagynak. Nem tartom elképzelhetetlennek azt, hogy működjön, csak valószinűtlennek. Túl könnyű szarba lépni, mint egy gyanútlan túristának a nyóckerben.
Mindenesetre a tavalyi év végére eldöntöttem, hogy olyan munkát akarok, ahol ezt veszélyt kiküszöböltük. Az elműlt egy évben ilyen helyen dolgoztam. Nyugodt volt a hangulat, bár pár alkalommal rendesen bele kellett húzni, végül mégis kényelmesen elértük a határidőket, az ügyfél boldog és nagyon jó fej velünk. Nekem ez bevállt és megtartom ezt az irányelvet: amíg találok olyan munkát ahol nincs potenciális probléma, addig olyat vállalok!

Code Review: Good Bye!