Kodgranskning

Kodgranskning har två syften som ofta blandas ihop: hitta fel och sprida kunskap. Båda förutsätter att granskningen är läsbar och konkret.

Granska i den här ordningen — det dyraste felet först:

1. Löser ändringen rätt problem? Fel lösning på rätt problem är dyrare än en bugg.
2. Finns korrekthetsfel? Gränsfall, race conditions, felhantering, None.
3. Finns säkerhets- eller integritetsrisker? Injektion, hemligheter, loggade personuppgifter.
4. Finns tester som fångar det viktiga?
5. Är den läsbar? Namn, struktur, döda grenar.
6. Stil — ska skötas av formatterare, inte av människor.

En granskare som börjar med punkt 6 har missat poängen.

Kommentarer som fungerar är specifika, motiverade och gärna med förslag:

❌ «Det här är fel.» ✅ «Rad 47: om lista är tom blir sum/len en ZeroDivisionError. Förslag: returnera 0.0 eller lyft ValueError — vad förväntar sig anroparen?»

Märk ut allvarsgraden så att författaren vet vad som blockerar:

Prefix	Betyder
blockerande:	måste åtgärdas före merge
förslag:	bör övervägas, blockerar inte
fråga:	jag förstår inte, förklara
nit:	smakfråga, ignorera gärna

Storleken avgör kvaliteten. Granskningar av PR:er över ~400 rader hittar betydligt färre fel per rad — granskaren tröttnar. Dela upp stora ändringar.

Att ta emot: svara på varje punkt, ändra eller motivera varför inte, och tacka. Koden är inte du. Den som blir defensiv får sämre granskningar nästa gång — och därmed fler buggar i produktion.

Vad som inte hör hemma i mänsklig granskning: formatering (formatterare), oanvända importer (linter), enkla typfel (mypy). Automatisera dem och lämna människan åt det som kräver omdöme.

Granska den här diffen:

+def hamta_anvandare(user_id):
+    q = f"SELECT * FROM users WHERE id = '{user_id}'"
+    rad = db.execute(q).fetchone()
+    log.info(f"hämtade användare {rad}")
+    return rad

Tre fynd, i allvarsordning:

1. blockerande (säkerhet): SQL-injektion — user_id interpoleras direkt i frågan. Använd parametriserad fråga: db.execute("SELECT * FROM users WHERE id = %s", (user_id,)).
2. blockerande (integritet): hela användarraden loggas, sannolikt med personuppgifter. Logga bara ett id.
3. förslag: SELECT * gör koden beroende av kolumnordning och hämtar mer än nödvändigt. Ange kolumnerna.

Alla tre är konkreta, motiverade och åtgärdbara — och två av dem hade en linter aldrig hittat.