Home / Blog / Softwareontwikkeling / Hoe kun je als programmeur je code reviews verbeteren met Google?
Zoals al eerder te lezen is in onze voorgaande blogpost heeft of wil elk zelfrespecterend software development team een code review proces. Het verbeteren van de softwarekwaliteit en versoepelen van het code review proces is niet alleen iets waar de reviewer voor moet zorgen. PR auteurs, ofwel programmeurs, moeten hier ook aan meewerken.
Google heeft namelijk óók voor de auteurs van PR’s een best practices lijst samengesteld. Ze beschrijven hierin uitgebreid waar je op moet letten als auteur/programmeur. Hierbij gaat het vooral om de communicatie naar de reviewer toe en hoe je als auteur de PR moet voorbereiden.
In dit artikel behandelen we de auteur-specifieke best practices van Google, en vullen we die aan met onze eigen kennis. Deze best practices hebben we bij Senet nu ook in gebruik genomen en ze hebben ons geholpen om effectievere code reviews uit te kunnen voeren. Ben jij een programmeur en ben je miscommunicatie over PRs zat? Of wil je betere PR’s aanleveren? Lees dan vooral verder en kom erachter hoe Google jou kan helpen PR’s beter voor te bereiden voor een reviewer!
Het is essentieel om je PR goed te beschrijven. Het geeft je een kans om aan de PR reviewer te beschrijven wat het doel is van je wijziging en waarom je bepaalde keuzes hebt gemaakt. Hiermee voorkom je eventuele eerste vragenrondes en daarmee versnel je het review proces.
Een best practice is om een vast formaat te gebruiken voor deze beschrijvingen. Google geeft aan dat je de beschrijving het beste kunt beginnen met een enkele, korte zin die beschrijft, op hoog niveau, wat je voor wijzigingen aanbiedt. Daarna kun je een wat uitgebreidere beschrijving toevoegen die op ‘low-level’ wijze beschrijft welke wijzigingen toegepast worden en waarom. Dit stimuleert je ook om goed na te denken over de wijzigingen die je hebt aangebracht.
De wijzigingen moeten op informatieve wijze beschreven worden en niet bestaan uit een samenvoegsel van commit messages of nietszeggende korte beschrijvingen. ‘Fix build’, ‘Fix bug’ of ‘asdf’ zijn geen informatieve beschrijvingen. Om alles nog te verduidelijken, koppelen wij onze originele aanvraag (ticket) aan een PR. Hiermee kan de reviewer ook de originele vraag controleren tegenover de opgeleverde PR.
Kleinere PRs zijn, op alle vlakken, toegankelijker en dus ook sneller te behandelen. Dit is fijner voor zowel de PR auteur als voor de PR reviewer. Een kleine PR is namelijk:
Dit zijn de redenen die Google benoemt welke ik volledig kan beamen. Het is ook zeker voor het teamgevoel veel beter als zoveel mogelijk wijzigingen met zo min mogelijk afgekeurde PR’s doorgevoerd worden. Je komt hierdoor als team in een bepaalde flow die de opleversnelheid en de flexibiliteit tussen teamleden bevordert.
Maar waar trek je de grens? Wat is klein of groot? Dat is een vraag die je niet voor alle teams kunt bepalen. Niet elk team is even technisch onderlegd, of heeft dezelfde tooling ter beschikking. Een grove regel, volgens Google, is dat een PR van 100 coderegels voldoet qua grootte en dat 1000 regels teveel is. Dit gaat echter natuurlijk niet altijd op. Als programmeurs en reviewers genoeg vertrouwen hebben in een refactor tool, zoals die in PHPStorm, IntelliJ of Visual Studio, kun je het hernoemen van een functie in een PR waarbij meer dan 200 bestanden bewerkt zijn goed laten keuren, mits de standaard test en build procedures dat toestaan.
Het gevaar van feedback is dat het snel persoonlijk opgevat kan worden. Zeker als het zaken zijn waar je het misschien niet direct mee eens bent of als je de reden erachter niet begrijpt. Houd dan in je achterhoofd dat het code review proces gaat om het verbeteren van kwaliteit van code. Dit kan de PR reviewer niet alleen, maar juist alleen met het gehele team. Als je dus feedback of commentaar krijgt op je PR is dit niet om jou als auteur lastig te vallen, maar juist omdat de reviewer denkt dat jouw code hiermee beter wordt.
Probeer ook nooit, in welke situatie dan ook, boos te reageren op stukken commentaar. Blijf professioneel, concreet en constructief. Dit geldt voor de auteur, maar ook voor de reviewer. Als je er niet uitkomt, zorg er dan voor dat je bij elkaar gaat zitten en het samen oplost. Doe dit ook als er niet-constructief commentaar op je PR wordt gegeven. Dit hoeft niet aan jou te liggen, maar het kan ook zo zijn dat de reviewer zijn commentaar minder goed beargumenteerd heeft.
Verder, als een reviewer vraagt wat een stuk code doet, zorg er dan altijd voor dat je de code duidelijker maakt. Als de reviewer de code niet direct snapt, kun je ervanuit gaan dat volgende programmeurs de code ook niet direct snappen. Dit kan in de toekomst resulteren in bugs doordat andere programmeurs wijzigingen aanbrengen die niet in de context passen van jouw onbegrijpbare code.
Tot slot is het altijd fijn om een nieuwe PR in te leveren en te denken dat je klaar bent. Dit kan dan natuurlijk flink tegenvallen als er alsnog commentaar wordt gegeven op je PR. Dit kan dan overkomen alsof de reviewer je onnodig wil tegenwerken terwijl jij vind dat hij gewoon zou moeten mergen. Probeer dit gevoel echter tegen te gaan. Ga bij elk stuk commentaar na, wat de reviewer zou bedoelen en of hij gelijk heeft. Controleer alles eerst zelf en ping-pong niet onnodig een afgekeurde PR terug. Mocht je er echt niet uitkomen omdat je het niet met het commentaar eens bent (na je eigen controle) zorg dan dat je beargumenteerd waarom jouw manier beter is (technisch, functioneel en voor de eindgebruikers).
Als je een PR inlevert, is de kans aanwezig dat je iets hebt mee gecommit dat:
Dit zou betekenen dat een reviewer je PR binnen 1 minuut kan afkeuren. Kijk daarom je eigen PR eerst eens goed na voordat je hem werkelijk inlevert. Probeer dan ook je eigen code vanuit het perspectief van de reviewer na te kijken. Daar haal je vaak veel leermomenten uit, nog voordat de reviewer er naar heeft kunnen kijken. Laat ook aan de reviewer merken dat je dit hebt gedaan. Verwerk commentaar niet direct als commits in je eigen branch, maar voeg net zoals de reviewer, commentaar toe aan de code toe via de code review tool. De reviewer kan zien dat je er extra aandacht aan besteed hebt en zal dit waarderen.
Ben je benieuwd hoe je dit het beste kunt doen? Lees dan ook onze blogpost over hoe je als reviewer het beste PR’s kunt nakijken.
Ben je benieuwd naar meer details over ons review proces en hoe wij onze programmeurs PR’s laten voorbereiden? Of, heb je nog geen review proces en wil je hier beter over geïnformeerd worden? Neem dan vooral contact met ons op zodat we mee kunnen denken over een maatwerk review proces dat naadloos bij jouw team(s) aansluit!
We nemen contact met u op!
Senet Eindhoven Gestelsestraat 258 5654 AM Eindhoven Bekijk op kaart
+31(0)40-2930395
KvK nummer: 17115078 Btw nummer: NL807989083B01
