Pôvodne publikované na opensource.com.
Ako Project Team Lead pre OpenStack Quality Assurance bolo code review mojou každodennou prácou. Nie príležitostne. Každý deň. Desiatky patchov týždenne naprieč veľkou kódovou základňou, od prispievateľov od prvýkrát skúšajúcich až po veteránov s desaťročnou praxou. Po dostatočne dlhej dobe si vytvoríte mentálny checklist. Toto je môj.
Chcem hneď na začiatku zdôrazniť: nemusíte byť jazykovým expertom ani projektovým veteránom, aby ste dali hodnotnú recenziu. Code review je o tom, priniesť čerstvý pohľad a nový uhol. Hlasovací systém v open source projektoch vám umožňuje vyjadriť názor bez absolútnej autority; nezhoda medzi recenzentmi nie je zlyhanie, takto vzniká dobrý kód. Jediná zlá recenzia je žiadna recenzia.
10 krokov
1. Pochopte zmenu
Začnite commit správou. Aký problém tento kód rieši a prečo na tom záleží? Ak tento kontext chýba alebo je nejasný, opýtajte sa naň skôr, než pôjdete ďalej. Recenzovanie kódu bez pochopenia jeho účelu je spôsob, ako premeškáte dôležité problémy a fixujete sa na triviálne.
2. Zvážte alternatívne prístupy
Skôr než sa ponoríte do detailov, opýtajte sa seba: ako by som to riešil ja? Ak je vaša odpoveď zmysluplne odlišná, nastoľte to, nie ako odmietnutie, ale ako otázku. Niekedy autor váš prístup zvažoval a z dobrého dôvodu ho odmietol. Niekedy naň nepomyslel. Tak či onak, konverzácia je užitočná.
3. Spustite kód
Vykonajte ho. So zmenou aj bez nej. Ak nie je možné spustiť celý projekt, izolujte funkciu a otestujte ju so simulovanými dátami. Čítanie kódu a spúšťanie kódu sú rôzne činnosti, ktoré zachytávajú rôzne problémy. Nevynechávajte to druhé len preto, lebo to prvé bolo v poriadku.
4. Posúďte potenciálne rozbitie
Premyslite okrajové prípady. Bude to fungovať naprieč všetkými podporovanými konfiguráciami? Sú nové závislosti kompatibilné so staršími prostrediami? Zavádzá zmena nejakú bezpečnostnú plochu? Budú sa možnosti príkazového riadku správať správne všade, kde sa používajú? Toto je krok, kde sa skúsenosť s projektom vypláca: viete, kde sú skryté riziká.
5. Zhodnoťte štýl kódu a efektivitu
Skontrolujte, či riešenie dodržiava zdokumentovaný štýl a konvencie projektu. Väčšina zrelých projektov má štýlovú príručku (OpenStack má napríklad HACKING.rst). Konzistentnosť s existujúcou kódovou základňou záleží. Patch, ktorý rieši problém, ale vyzerá, ako keby patril do iného projektu, vytvára technický dlh.
6. Overte, že nie sú žiadne nepoužívané premenné ani importy
Každý nový import a každá nová premenná by mali slúžiť účelu. Toto sa ľahko prehliadne, najmä vo väčších patchoch, kde sa niečo pridá na začiatku a neskôr sa nahradí. Lintovanie nástrojmi ako flake8 väčšinu z nich pre Python zachytí, ale nespoliehajte sa len na automatizáciu: linter vám nepovie, že import sa síce používa, ale nemal by sa.
7. Skontrolujte duplicitnú funkcionalitu
Pred schválením novej funkcie skontrolujte, či podobná logika už existuje inde v projekte. Duplikácia je jedným z najtrvalších zdrojov technického dlhu. Ak autor nevedel, že existujúci nástroj tam bol, povedať mu to je skutočný príspevok, nie kritika.
8. Vyžadujte jednotkové testy
Nové funkcie a zmeny logiky by mali byť sprevádzané testami. Toto je rozumná požiadavka, ktorú treba konzistentne dodržiavať. Ak patch pridáva zmysluplné správanie bez testu, požiadajte o jeden. Odpoveď na otázku "ako vieme, že to funguje?" by nemala byť "prečítali sme kód a vyzerá správne."
9. Overte úplnosť refaktoringu
Keď sa kód refaktoruje (premenuje sa funkcia, odstráni sa parameter, zmení sa vzor), každý výskyt v kódovej základni musí byť aktualizovaný. Použite grep alebo váš vyhľadávací nástroj. Refaktoring, ktorý je len napoly aplikovaný, je horší ako žiadny refaktoring: zanecháva kódovú základňu v nekonzistentnom stave, ktorý bude potichu rozbíjať veci.
10. Skontrolujte, či dokumentácia potrebuje aktualizáciu
Ak zmena ovplyvňuje správanie viditeľné používateľom (nové možnosti, zmenené predvolené hodnoty, odstránená funkcionalita), dokumentácia by to mala odrážať. Toto je krok, ktorý sa najčastejšie vynecháva a spôsobuje najviac zmätku pre používateľov šesť mesiacov neskôr.
Poznámka o spôsobe poskytovania spätnej väzby
Technická správnosť je len časť toho, čo robí recenziu užitočnou. Záleží aj na tom, ako spätnú väzbu komunikujete. Kladte otázky namiesto vynášania rozsudkov. Vysvetľujte svoje uvažovanie namiesto samotného poukazovania na problém. Keď si nie ste istí, že niečo je zlé, povedzte to: "Nie som si istý týmto, ale uvažoval si nad..." je perfektný komentár recenzenta.
Cieľom recenzie nie je nájsť chybu. Je to zlepšiť kód a pomôcť autorovi zlepšiť sa. Tieto veci spolu súvisia, ale sú rozdielne. Recenzia, ktorá zachytí každý technický problém, ale zanechá autora s pocitom porážky, zlyhala v tom druhom. V komunite, ktorá funguje na dobrovoľnom úsilí, na tom záleží.