Code Review kokouksen hyvät ja huonot puolet

Code Review PHZ Full Stack:lla

Lean 8 hukkaa (waste)

Ketterän ohjelmistokehityksen kuten Extreme Programming, Scrum ja Kanban perusajatukset pohjautuvat 80-luvun Lean-ajatteluun, joka taas alun perin japanilaisten amerikkalaisilta kopioimasta sarjatuotantoajattelusta 20-luvulta mm. Ford:n T-mallin tuotannosta. Yksi Lean perusfilosofioista on hukan (waste) ja turhan resurssienkäytön pienentäminen minimiin, jos kyseinen vaihe tai toimenpide ei lisää koko järjestelmän suorituskykyä. Leanin 8 hukkaa ovat ylituotanto, odottelu, (tavaran tai ihmisten) liikuttaminen, uudelleentyöstö (tehdään sama asia uudestaan) ja lisäarvoa lopputuotteelle tuottamaton työ, varasto (julkaisemattomat ominaisuudet), ihmisten (kyvykkyyksien) alikäyttö, viat (tarkistus ja korjaus) sekä ylikapasiteetti.

Koodikatselmukset

Koodikatselmus (code review) on ohjelmistotuotantokäytäntö, joka toteuttaa corporate governance auditointikäytännön, missä joku muu kuin lähdekoodin tekijä tarkistaa tehdyn työn. Katselmointi voi olla formaali, kuten esim. PCIDSS -luottokorttistandardin mukaan tulee tehdä, ja sen voi toteuttaa joko yksi tai useampi henkilö. Koodikatselmusten tavoitteena on laadun parannus, vikojen löytäminen, osaamisen siirto ja oppiminen, kollektiivisen koodin omistuksen ja jaetun vastuun (full stack) mahdollistaminen, parempien toteutusideoiden löytäminen ja jakaminen sekä standardien noudattaminen (esim. ISO/IEC -standardit).

Koodikatselmukset eivät varsinaisesti ole Extreme Programming -menetelmän käytäntö, koska XP:ssä katselmus on parityössä jatkuvaa. Erillinen koodikatselmus voidaan siten nähdä joko Lean Waste:na johon kuluu turhaan työtunteja, tai sitten sillä yritetään korjata ideaalisen prosessin puutteita. PHZ Full Stack:lla koodikatselmuksia tehdään XP-prosessin lisänä lähinnä harjoittelutarkoituksissa, koska ne ovat yleisiä asiakasprojekteissa, sekä Full Stack -osaamisen ja projektien välisen standardoinnin nopeuttamiseksi. Tämän takia PHZ Full Stack:lla koodikatselmuksen tulisi tehdä tiimin jäsen joka tietää vähiten kyseisestä ohjelmointikielestä tai järjestelmän osasta (esim. harjoittelija lead developer:n sijaan).

Koko tiimin koodikatselmuskokous

Joissakin projekteissa on käytössä esim. kerran päivässä tai viikossa tapahtuva koodikatselmuskokous, johon koko tiimi osallistuu. Koodin tekijä voi esitellä tekemäänsä koodia muulle tiimille rivi riviltä selittäen sen suunnitteluperiaatteet ja tarkoituksen, ja muut tiimiläiset voivat kommentoida sitä. Tämän käytännön hyviä puolia ovat tiedon jakaminen, standardoinnin lisääminen sekä vikojen löytämisen maksimointi. Huonoja puolia ovat erittäin suuri työajan käyttö, sillä koko tiimi voisi olla samaan aikana tekemässä myös uusia ominaisuuksia. Toinen huono puoli katselmointikokouksessa on viiveet, joita se aiheuttaa palaute- ja korjaussykliin. Verrattuna tilanteeseen, jossa katselmus suoritetaan välittömästi sen avaamisen jälkeen tai vähintään heti seuraavan päivän aamulla ensimmäisenä työtehtävänä, ohjelmoija (ja asiakas) joutuu odottamaan seuraavaa katselmuskokousta (joka pahimmillaan järjestetään iltapäivällä) esimerkiksi perjantaista maanantai iltapäivään. Mikäli projektilla ei ole aikataulupaineita tai järjestelmä on tuotannossa, tämä voi olla äärimmäisen stressaavaa asiakkaalle, joka kuumeisesti odottaa tuotantopäivitystä kriittisten ongelmien korjaamiseksi. Tämän jälkeen ohjelmoija saa katselmuskokouksesta palautteen, ja voi tehdä korjauksen seuraavan päivän katselmuskokoukseen, jolloin korjaus voi päätyä asiakkaan testaukseen vasta nopeimmillaan 6 päivän päästä sen aloittamisesta.

Koodikatselmukset ensimmäiseksi

Lean 8 hukan näkökulmasta koodikatselmuksia ideaalisesti ei olisi järjestää ylipäänsä, koska ne ovat lähtökohtaisesti 7. hukkaa eli vikojen tarkistuskuluja. Zero Defects -ideologiassa ja mm. Poka-Yoke -virhevarmennuskäytännöillä  voidaan pyrkiä eliminoimaan kaikki virhelähteet siten, että katselmuksista ja testausvaiheista voidaan luopua kokonaan. Lean:lle vaihtoehtoisen Total Quality Management tilastotieteeseen nojautuvan tilastollisen prosessikontrolli-ideologian mukaisesti voidaan myös esim. Six Sigma -käytännöillä analysoida ja poistaa tilastollisesti virhelähteitä.

Kuitenkin, jos koodikatselmuksia kuitenkin tehdään, hukan minimoimiseksi katselmusprosessi tulisi aloittaa välittömästi (ilman viivettä) sen jälkeen kuin sen pull request on avattu. Esim. USA:n Sarbannes-Oxley (SOX) -lain auditointiperiaatteen pörssiyrityksille täyttää, jos yksi katselmoija tarkistaa koodin. Ideaalisesti tarkastaja ei ole aina sama, vaan kiertää jatkuvasti tiimissä.

Kommenttien formaalin dokumentoinnin ja työajan minimoimisen näkökulmasta reaaliaikaisen kokouksen sijasta on parempi, että kaikki katselmointikommentit kirjoitetaan versiohallintaohjelmistoon webbisivulle, jolloin tarkastuksesta jää myös kirjallinen muistio.

Koodikatselmukset PHZ Full Stack:lla

PHZ Full Stack:lla päivittäiseksi työjärjestykseksi on asetettu edellä mainittujen syiden takia seuraava työjärjestys:

1. Päivä alkaa Daily Standup:lla
* Wastea on, että kaksi tai useampi henkilö katselmoi samanaikaisesti samaa pull request:ia, joten katselmuksien tekeminen pitäisi jakaa
2. Devops Problem Management
* Jos tuotannossa esiintyi yön aikana ongelmia (palvelin on kaatunut), tätä lähdetään selvittämään ensimmäiseksi tilanteen normalisoimiseksi ja siirtämiseksi normaaliin työjonoon (kohtaan 8)
3. Muuten ensimmäiseksi katselmoidaan muiden kuin itse tekemät pull requestit
* Ei siis aloiteta koodaamaan, vaan PHZ Full Stack:n arvojen mukaisesti annetaan ensiksi palautetta muulle tiimille
4. Korjataan CI-pipeline virheet
* Esim. coding convention -ongelmat automaattitarkastuksesta
5. Korjataan yksikkötestit
6. Korjataan hyväksyntätestit
7. Viedään julkaisematon koodi testaukseen/tuotantoon
8. Aloitetaan uuden ominaisuuden toteuttaminen
* Vasta kun kaikki edellämainitut on tehty, koska muuten edelliset vaiheet aiheuttavat hukkaa (wastea).