Technologas #12: Kaip sukurti pull-requestą, kuris nudžiugins visus?

Vaidas

Vaidas, CTO

Dobro! Turbūt retai sutiksi programuotoją (o jei sutiksi, tai jis bus 👽), kuris nėra atlikęs pull-request peržiūrų. Patenku tarp tų laimingųjų, kurie tai daro beveik kasdien, ir šioje vietoje ne kartą iškilo klausimas – bet ar aš peržiūriu kiekvieną PR teisingai?

Ko gero, ne visada. O ką tuomet galėčiau daryti geriau, kad progresas būtų naudingas visam projektui, o ne tik momentinių poreikių įgyvendinimui? Tad šiandien ir noriu pakalbėti apie pilną PR spektrą: nuo pernelyg išsamaus ir apipinto komentarais iki labai nemėgstamo iki atlikto tik tam, kad kodas išvažiuotų į produkcinę aplinką. 

Reikia čia ir dabar

Toks PR labai retas, bet kai pasitaiko, reikia greitai priimti sprendimą, kuris gali būti tiek teigiamas, tiek neigiamas. Susitikti su tokiais man teko ne kartą, bet po to dažnai tekdavo matyti ir antrą, ir trečią išplaukiantį PR’ą. Tarkime, neveikia tam tikras mygtukas pagrindiniame puslapyje ir mano kolegai reikia jį sutvarkyti. Per skubėjimą jis palieka klaidą ir aš, kadangi reikia čia ir dabar, sutinku su juo. Atliktus pakeitimus galima išleisti į produkcinę aplinką, po to seka klaidos atradimas, pataisymas ir vėl išleidimas. Liūdna.

(Per) dideli pakeitimai

Pasidalinsiu, kaip aš jaučiuosi, peržiūrėdamas tokius PR’us. Atsidarai ir greitai supranti, kad čia bus reikalų. Bandai peržiūrėti. Pirmi porą failų – viskas ore. Bet po kelių minučių išgirsti tokį pelės ratuko garsą ir nusprendi užbaigti reikalą dienos pabaigoje. Kai pasibaigi likusius darbus, ateina eilė sugrįžti prie didžiojo PR. Grįžęs jau gerai nebesupranti, kur baigei, tai pradedi tikrinti nuo galo. Nedaug laiko tepraeina, kol galiausiai nusispjauni, ir arba sutinki, arba tiesiog ignoruoji.

PR’as, kurio peržiūrėti nereikia

Tokie užknisa labiausiai. Jie atsiranda dėl įvairių priežasčių: kartais dėl funkcijos, kartais dėl to, kad pasiliekama viską išleisti vieną dieną. Turbūt ne vienam atėjus į naują projektą yra tekę išgirsti: “Šitas PR yra neperžiūrimas, nes jau kabo ~1 mėn, mes į jį jungiame kitas šakas”. 

Ir man visada buvo keista, kaip galima išleisti į produkcinę aplinką tiek pakeitimų. Suprantu, kad kiekvienas pakeitimas buvo pratestuotas. Suprantu, kad kiekvienas komitas buvo peržiūrėtas. Bet ar ne lengviau turėti funkcijų įjungimą/išjungimą (feature flags)? Nes išėjus į produkcinę aplinką klaidos tikimybė išauga ir čia jau atrodo geresnis kelias “sulūžk-greitai” (fail-fast).

Kam tie aprašymai?

Yra nemaža dalis pull-requestų, kuriuos atsidarius nelabai aišku, ką žmogus jais nori padaryti ir kodėl jų reikia. Kokie tai pakeitimai? Ką šis PR testuoja? Ar tai vabaliukų gaudymas, ar funkcija? Ar tai sugriauna kokią nors egzistuojančią funkciją? Ar šis pakeitimas pilnai ištestuotas? Ši informacija turėtų būti aprašyta PR’e.

Jeigu ne, visuomet galima paprašyti atsakymų į šiuos klausimus arba nesutikti su PR. Be to, yra daugybė įrankių tokioms problemoms spręsti, pavyzdžiui, PR templates.

Ei, o kam tie PR’ai išvis? Galima juk ir be jų dirbti

Tokia neigiama versija irgi įmanoma. Tokią reakciją išprovokuoti gali įvairios praktikos: neatsargus grįžtamojo ryšio išsakymas intravertui, paskutinių dienų pakeitimai prieš sprinto galą būna ir taip įtempti, o dar prisideda kodo peržiūros. Ne tai kad peržiūros niekam tikusios, greičiau reikėtų peržiūrėti patį procesą ir išsiaiškinti, kas blogo nutinka ten.

Svarbu ne tik pats PR, bet ir ką su juo darome

Koks tas requestas bebūtų – didelis, mažas ar net nelabai svarbus – jį peržiūrėti vis tiek reikia. Nors ir drąsiai pasisakome, kad žiūrėsime PR’us kiekvieną dieną, bet realiame gyvenime dažniausiai stringame, kai prieiname prie planų vykdymo. Visa tai man priminė vieną internetuose matytą paveikslėlį:

Šioje vietoje kartais ir pats jaučiu kaltę. Dažnai komandose pastebiu tokį triuką: retrospektyvose žmonės pasako, jog negerai, kai PR’ai neperžiūrimi laiku. Reikalai tam kartui pagerėja, bet šitas pasitaisymas įvyksta labai trumpam. Po mėnesio ar dviejų ateini į kitą retrospektyvą ir girdi tą pačią bėdą vėl iš naujo.

Neseniai skaičiau apie tokią taktiką – jei neperžiūri PR'o daugiau nei vieną parą, botas parašo į Slack #board_of_shame kanalą. Man kalimas prie kryžiaus niekada neatrodė kaip gera ir motyvuojanti priemonė, bet kiekvienam savi metodai. Jei turite kitų sprendimų, palengvinantį šį kelią, visuomet laukiu jų komentaruose.

Stiliukas

Kartais mes pervertiname dalykus, o stilius yra vienas iš jų. Nors tai galioja ir rūbams, šįkart ne apie juos. Kitoks kodo stilius ne visuomet yra blogas pasirinkimas. Kartais komentuodami tik stilių nusižengiame pagrindiniam peržiūros tikslui – kad į produkcinę aplinką išeitų kodas, su kuo mažiau vabalų. Stiliaus problemoms išspręsti yra ir geresnių būdų.

Visgi daug svarbiau negu kodo stilius yra suprasti, kas jame parašyta ir ką tie pakeitimai padarys. Negerai jeigu kodą peržiūri žmogus, nedirbantis prie to projekto, nes neturėdamas gilesnio supratimo tik apie stilių jis ir galės komentuoti.

O tai kaip turėtų būti?

Aišku, galima visą dieną baksnoti pirštu į kreivus PR’us, bet mokykloje mokė, kad kritikuojant reikia duoti ir kažką konstruktyvaus. Išgliaudžius visus "bet, o, tačiau" prieiname prie to normalaus PR’o, kuris turėtų būti:

  • Tvarkingai aprašytas: pavadinimas yra aiškus ir glaustas, aprašyme yra nuoroda į užduotį, ir aiškiai aprašyti visi pakeitimai.
  • Ne per didelis (kad atspausdinus git’o diff’as tilptų į 4 A4 formato lapus).
  • Stilius patikrinamas įrankiais, o ne pasikliaujama žmogaus intervencija.
  • Jį peržiūri ir patvirtina per atitinkamai mažą laiką.
  • Susidėti iš ne daugiau nei 3 komitų (arba git’o repositorija turėtų sutraiškyti juos).

Tokia ta teorija, bet ar daug tokių pull-requestų jums teko padaryti praktikoje?

Vaidas

Vaidas, CTO

Technologijų entuziastas, nuolat ieškantis būdų, kaip optimizuoti procesus ir kaip atrasti dar neišbandytą naminio sidro skonį.