Review & Team Happiness
Vorsicht Aufzeichnung!
Warum sollte mich das interessieren?
Effektiver lernen
- Fokus auf Themen an denen man wachsen kann
- Es geht nicht um Stylingänderungen
- Lernen durch Annehmen von konstruktiver Kritik
Das Team wird besser
Review als:
- Abgleich und Lern-Chance zwischen dem Team
- Gelegenheit für alle anderen im Team, die sich da dran ein Beispiel nehmen
Weniger Reibung
- Codereviews sind Reibung
- Sich bewusst mit ihnen zu beschäftigen minimiert diese
Wertschätzung
Euer Reviewer
- hat nicht unendlich Zeit
- ist nicht euer Qualitätssicherungs-Experte
- ist kein Hindernis, das überwunden werden muss
Methoden
Selber Reviewen
- Bevor MR versendet wird
- Stell dir vor, du kennst den Code noch nicht
- Benutze dasselbe Environment wie der Reviewer
MR Beschreibung
- Klare Einfuehrung in den Context
- Screenshots? Videos?
- Ticket verlinken
- Hohe Flugebene
Automation
Automation
- Style
- Lint
- Compile
- Tests
- Nie vorher schon zum Review abgeben
Verständnisfragen
- In der Regel ein Zeichen für suboptimalen Code
- Code verständlicher neu schreiben
- Bei Problemen in Zukunft wird der Code und das Gitlog referenziert
- Keine 1:1 Kommunikation
Do one thing
- Ein MR sollte sich um ein Thema kümmern
- Kleine und einfache Änderungen
- Kein "Ich mach noch mal eben das mit"
Refactoring
vs
Behavior change
- Nicht vermischen
- Sonst hart zu reviewn
Beispiel Commitaufteilung:
- Tests für vorhandenen Code hinzufügen
- Refactoring
- Änderung des Verhaltens und Anpassung der Tests
Refactoring
vs
Behavior change
"Zeig mir einen Merge Request mit 1000 Zeilen und ich finde 1 Fehler"
"Zeig mir einen Merge Request mit 10 Zeilen und ich finde 1 Fehler"
Große Changes aufteilen
- Ab circa 400 Zeilen
- Versuchen logische Pakete zu finden
-> Besseres Feedback & glücklicher Reviewer
Etiquette
- Be nice - benutzt Emojis
- "Welche Änderungen wären hilfreich?"
Kritik
- Probleme finden ist gut
- Reviewer und MR Sender sind im selben Team
Nachsicht mit Reviewer
- Können sich genauso irren
- Können Code falsch verstehen
- Sind nur Menschen
- Ist der Grund für das Missverständnis im Code?
MR Zustand
Team muss Konventionen haben:
- Es sollte immer klar sein, wer einen MR bearbeitet
- Wann Comments resolven?
- Kommentar mit "Done"
- Oder Kommentar warum Änderung nicht gut wäre
- Resolve durch Reviewer
How to git commit
Betreff vom Body trennen
- Betreff mit bis zu 72 Zeichen, besser 50
- Betreff immer Capitalized
- Betreff nicht mit Punkt
- Nicht jeder Commit braucht nen Body
Derezz the master control program
MCP turned out to be evil and had become intent on world domination.
This commit throws Tron's disc into MCP (causing its deresolution)
and turns it back into a chess game.Imperativ
- Refactor subsystem X for readability
- Update getting started documentation
- Remove deprecated methods
- Release version 1.0.0
Imperativ
Hilfestellung:
- If applied, this commit will your subject line here
Body
- Maximal 72 Zeichen pro Zeile
- Erkläre Was und Warum nicht Wie
commit eb0b56b19017ab5c16c745e6da39c53126924ed6
Author: Pieter Wuille <pieter.wuille@gmail.com>
Date: Fri Aug 1 22:57:55 2014 +0200
Simplify serialize.h's exception handling
Remove the 'state' and 'exceptmask' from serialize.h's stream
implementations, as well as related methods.
As exceptmask always included 'failbit', and setstate was always
called with bits = failbit, all it did was immediately raise an
exception. Get rid of those variables, and replace the setstate
with direct exception throwing (which also removes some dead
code).
As a result, good() is never reached after a failure (there are
only 2 calls, one of which is in tests), and can just be replaced
by !eof().
fail(), clear(n) and exceptions() are just never called. Delete
them.Tips
- GIT via Commandline
- GIT rebase verinnerlichen
- Vim
- Jeder (zwischen) Commit builded
Quellen
- https://mtlynch.io/code-review-love/
- https://chris.beams.io/posts/git-commit/
git reviewer happiness
By Kolja Lampe
