Bästa praxis för kodgranskning

Författaren av att få ett annat perspektiv på sin uppgift och att få kod. De lär sig ofta nya knep eller upptäcker ett potentiellt mer optimalt sätt att lösa ett visst problem. De kommer också att distribuera sina ändringar med gott självförtroende, eftersom de vet att andra personer har kontrollerat att koden är korrekt och har kommit överens om att allt är bra.

Granskaren drar nytta av att se olika sätt att lösa problem i praktiken. De kommer också att förbättra sina färdigheter i kodläsning, vilket är mycket viktigt när man dyker in i t.ex. ett bibliotek som utvärderas för användning i en projekt. Kodgranskning är också en möjlighet att lära sig för granskaren lika mycket som för författaren: de kan mycket väl lära sig nya knep också.

Den Team som helhet gynnas eftersom en granskning av en lösning på ett visst problem kräver att man förstår problemet åtminstone på en hög abstraktionsnivå. Detta bidrar till att riva ner eventuella oavsiktliga kunskapssilos som kan uppstå i ett team. Det ökar också "bussfaktorn": eftersom minst två personer (helst fler) är medvetna om en viss förändring är sannolikheten mindre för en situation där ingen i teamet vet hur man uppdaterar en modul eller varför en viss bugg uppstår.

Kunden drar nytta av snabbt och säkert distribuerade ändringar och lösningar. Tillsammans med andra bästa metoder (t.ex. hög testtäckning, CI/CD, staging-miljöer etc.) säkerställer kodgranskningar också att det som distribueras är säkert, sunt och uppfyller kraven enligt specifikationen.

Avsikten med och användningen av dessa riktlinjer

Kom ihåg att dessa förslag framför allt syftar till att skapa en miljö som främjar ambitiös och effektiv problemlösning, samtidigt som de skapar ett skyddsnät och främjar förtroende och öppenhet hos varje medarbetare.

Även om det starkt rekommenderas att ett team följer dessa riktlinjer är de inte avsedda som fasta regler. Det här ramverket är inte heller avsett som en "process" som ska följas exakt, eftersom rigida processer tenderar att minska hastigheten och främja slöseri.

Du är mer än välkommen att bygga vidare på dessa riktlinjer inom ditt team. Kom dock ihåg att när utvecklare roterar mellan team kommer de att förvänta sig att kodgranskningen i alla team de ansluter sig till fortfarande baseras på detta dokument. Håll eventuella ytterligare regler dokumenterade och bidra med förbättringar som har fungerat exceptionellt bra till det här dokumentet.

Ansvarsområden kring kodgranskning

Alla i teamet har vissa ansvarsområden när det gäller kodgranskning. Nedan beskrivs vad man bör och inte bör göra när det gäller kodgranskning, uppdelat efter roll i processen.

Projektledare

• DO se till att dina arkiv är välkonfigurerade (t.ex. att sammanslagningar till din produktionsinriktade gren inte är tillåtna utan minst en godkännande granskning).
• DO se till att ditt team förstår och tillämpar dessa metoder, och arbeta aktivt för att främja förståelsen för varför vi gör saker på ett visst sätt.
• DO se upp för oavgjorda situationer där motstridiga åsikter inte kan lösas: som teknisk ledare för ditt team är det ditt ansvar att välja den mest relevanta lösningen i sådana fall och se till att arbetet fortskrider.
• Men.., INTE använda projektledning som ett trubbigt instrument. INTE "pull rank". DO välkomnar granskning och kritik av dina lösningar lika mycket som du uppmuntrar dem på andras arbete.
• ÖVERVÄG att lägga till en GitHub-integration i teamets Slack-kanal. Det kan vara till hjälp för att bättre sätta granskningsförfrågningar på granskarnas radar, men beroende på den totala volymen i din kanal kanske detta inte är rätt för ditt team.

Medlem i teamet

• Delta i kodgranskningar. Det är inte acceptabelt att inte granska kod.
• Kom ihåg att göra dina kodgranskningar: dina lagkamrater är beroende av att du gör framsteg med deras arbete!
• Om du absolut inte kan göra en viss granskning, DO kommunicera det tydligt och öppet.
• Men.., INTE anta att du inte kan göra en viss granskning eftersom du inte känner till den modulen/sidan av systemet/affärslogikspecifikationen. Kodgranskning är en viktig inlärningsmöjlighet.
• Om du känner att du inte vet tillräckligt om något för att göra en recension, DO fråga författaren om det: de kommer gärna att förklara vad ändringarna är tänkta att göra.
• INTE neka recensioner baserat på erfarenhetsnivå (din eller författarens).
• DO försök att granska minst lika många PR som du producerar. Helst ska förhållandet mellan antalet givna granskningar och antalet nödvändiga granskningar vara över 1 (särskilt i större team).

Författaren

• DO förstå att det är ditt ansvar att få din kod granskad - ditt team kanske proaktivt letar efter pull requests att granska, men de behöver inte göra det.
• INTE alltid tilldela/begära granskningar från samma teammedlemmar - du kommer att dra större nytta av en varierad granskningspool (och omvänt kommer ett bredare spektrum av utvecklare att dra nytta av att granska din kod)
• INTE utesluta någon från granskning baserat på erfarenhet. Juniorutvecklare gynnas av att granska kod. Seniora utvecklare gynnas av att granska kod. Som det står i förordet till det här dokumentet, alla har nytta av att granska kod.
• ÖVERVÄG använda en slumpgenerator för att välja dina granskare. T.ex. i Ruby, %w[lagkamrat1 lagkamrat2 lagkamrat3].sample kan göra underverk.
• DO tilldela minst två granskare till dina pull requests, om det inte är absolut omöjligt. På så sätt drar fler människor nytta av processen (och med tre personer är det svårare att få oavgjort).
• DO vara lyhörd i dina pull requests. Även om du inte bör bryta ditt flöde för att svara direkt på alla kommentarer, se till att dina svar kommer i rätt tid - annars kommer dina PR att dröja kvar i kodgranskningen på obestämd tid.
• DO Ha en öppen attityd. Utgå alltid från att din granskare försöker hjälpa till med de bästa avsikter. Förklara din logik, bemöt granskarens argument och besvara hans eller hennes frågor.
• DO vara artig. Missförstånd kan uppstå, men de behöver inte gå överstyr och skada stämningen i teamet.
• DO Var ärlig. Om du anser att detta är den bästa lösningen, säg det och presentera dina argument. Om du blir övertygad om att din granskares förslag är bättre än det du har tagit fram, berätta det för dem. Om du tror att en "bästa av två världar"-lösning som använder både dina och din granskares idéer kan produceras, föreslå det för dem. I slutändan ska du arbeta för att uppnå konsensus i dina pull requests.
• DO Överlåt till din granskare att lösa deras kommentarer - lös dem inte bara för att du är övertygad om att det är bra nu.
• DO aktivt förklara din uppgift, ditt resonemang och andra krav för dina granskare. Det är okej att inte veta - det är inte acceptabelt att undanhålla kunskap.
• INTE tror att du kan allt - vi är alla fantastiska specialister, men det är viktigt att du har ett visst mått av ödmjukhet när du arbetar med dig själv.
• DO vara den första granskaren av din kod. Ta på dig en granskares hatt och skanna koden noggrant som du skulle göra för den person som du inte gillar mest. Identifiera och eliminera de mest uppenbara sakerna, som tomma rader, eventuella rester eller saknade specifikationer. Hoppa inte över något - det kommer med största sannolikhet att påpekas ändå. Slösa inte bort granskarnas tid!
• DO beskriv din pull request noggrant. Beskrivning är bra när granskaren inte kommer att bli överraskad av något när han läser koden. Kom ihåg att han inte kan läsa dina tankar. Det är därför det är viktigt att beskriva saker som inte är uppenbara, viktiga beslut med anledningen eller alla nya klasser och filer.
• ÖVERVÄG med hjälp av pull request-mallen. Om du använder Github, lägg till det i ditt arkiv under .github/pull_request_template.md. Det uppmuntrar alla teammedlemmar att beskriva sina pull requests. Det är också mycket enklare att skriva när du har ett beskrivningsfält som fylls i med en mall. Här kan du hitta en mall som vi använder i ett av våra projekt https://gist.github.com/weemanjz/a20ccb9f3f492b9bd21ab026a1d46353

Författaren

• DO förstår att det är ditt ansvar att se till att din kodgranskning - ditt team kanske proaktivt letar efter pull requests att granska, men de behöver inte göra det.
• INTE alltid tilldela/begära recensioner från samma kodgranskare - du kommer att dra större nytta av en varierad pool av granskare (och omvänt kommer ett bredare spektrum av utvecklare att dra nytta av att granska din kod)
• INTE utesluta någon från granskning baserat på erfarenhet. Juniorutvecklare drar nytta av att utföra kodgranskning. Seniora utvecklare drar nytta av att utföra kodgranskning. Som det står i förordet till detta dokument, alla fördelar med att utföra kodgranskning.

Recensent

• DO använd ett språk som ger förslag istället för krav. Istället för att säga "Du borde förbättra kodkvaliteten genom att göra X istället", säger "Har du funderat på att förbättra kodkvalitet genom att göra X?"
• DO förklara dina förslag. "Jag tycker att X är bättre här eftersom det hjälper till i kunskapsöverföring och förbättra kodkvaliteten."
• Även om ditt förslag kommer från objektiva källor (t.ex. kodstil riktlinjer), DO be författaren att göra något i stället för att säga åt dem att göra något. "Vänligen håll alla widgets frobbade enligt våra kodstil guide - [länk]"
• INTE anta att du vet allt. "Jag har förstått att den här widgeten aldrig ska frobba, och under dessa förhållanden kommer den att göra det - är det här ett undantag som behöver en kodgranskning?"
• DO använda ett inkluderande språk. "Jag tror att vi skulle få det bättre i framtiden om vi byggde det här på det här sättet. Vad tycker du om det här bättre kodgranskning förslag?" och "Kanske borde vi använda X här istället för en effektiv kodgranskning?"
• DO vara snabb med att göra dina kodgranskning. Du bör inte bryta ditt flöde för att göra dem, men försök att hålla slingan tät om det är möjligt. En del gillar att göra dem i början eller slutet av arbetsdagen, antingen som "uppvärmning" eller "nedvarvning".

Observera att dessa nyckelord har infogats på ett sådant sätt att textens sammanhang och koherens inte påverkas. Om du vill ha dem på något särskilt ställe, vänligen ange det.