Tipps für Organisation von Code-Reviews nach einem Pull Request.

[Zrebna]

Hallo!

Wie geht man bei Code-Reviews nach einem Pull Request via GIT, bei dem man als Reviewer gesetzt worden ist am Besten vor?

Natürlich kann man sich alle Changes im Browser ansehen - z.B. auf gitHub, BitBuckets, oder was man da auch immer intern verwendet.
Jedoch ist das oft nicht so angenehm und übersichtlich und man kann sich nirgends "reinklicken", wie in der IDE.
In der IDE selber kann man schon einen checkout auf den zu reviewenden Branch machen und dann via Git History einzelne Klassen vor und nach einzelnen Commits ansehen.

Aber was hier fehlt ist eine Art Stand vor dem Pull Request des gesamten Projektes (alle Klassen) im Vergleich mit dem Stand nach dem Pull Request.

Wie macht ihr das in der Praxis?
Evtl. einen eigenen Workspace für Code-Reviews anlegen und dort auf den zu reviewenden Branch auschecken ( = Stand Projekt nach allen Commits des zu reviewenden PRs) und die Situation dort mit dem Stand im eigenem Workspace vergleichen, also dem Stand ohne den Commits, die zum PR gehören.

Wobei dann nicht die Unterschiede "gehigh-lighted" werden.
Mich würde einfach interessieren, wie ihr das in der Praxis am Besten macht, weil von Euch sicher einige bereits in Rollen sind/waren, bei denen man viel Code-Review-Erfahrung sammelt und ggf. gute Tipps geben kann.^^

Lg
Zrebna

 

[bullish]

Immer auf Github reviewen, da kann man den PR-Ersteller auch gleich mitteilen, was falsch gelaufen ist.

 

[LimDul]

Wie üblich - Kommt drauf 🙂

Bei uns in der Firma sieht der Prozess so aus, das der Jenkins auch direkt die gesamten Anwendungen auf dem Stand baut und deployed. Das heißt ich kann den Stand auch Funktional testen ohne ihn auszuchecken. Dann hängt es im Endeffekt vom Umfang der Code-Änderungen.

Lokale Änderungen reviewe ich online (wir verwenden gerrit). Dort sehe ich das Diff, kann anmerken machen und auf dem gebauten Stand auch direkt funktional testen.

Änderungen die nicht mehr lokal sind, sondern sich über mehrere Teile erstrecken checke ich lokal in meiner IDE aus. Ich schaue da auch online mir das Diff an, aber in einigen Stellen wechsel ich dann in die IDE um mehr Kontext zu haben und auch mal links und rechts zu schauen. Relevant ist da auch - was einem kein Tool abnehmen kann - die Erfahrung. Den interessant sind bei solchen Changes auch immer die Stellen, die nicht angepasst wurde, aber hätten angepasst werden müssen. Die findet man nur mit Erfahrung und einem funktionalen Test. Den Browser mit dem Online Diff brauche ich aber eh immer, weil da muss ich ja alle Anmerkungen reinschreiben wenn was nicht passt.

 

[bullish]

Oh ja, Tests kann Github natürlich auch automatisch durchrattern lassen ... Aber da sind wir schon einen Schritt weiter, den bevor es überhaupt zu einem formalen Review kommt, müssen ja wenigstens die Tests bestanden sein.

 

[Zrebna]

Immer auf Github reviewen, da kann man den PR-Ersteller auch gleich mitteilen, was falsch gelaufen ist.

Klar, Kommentare gehen auf GitHub in den PR rein.
Aber der Codereview auf GitHub erscheint teils mühseliger als in der IDE Code durchzuklicken.

Wegen Tests:
Nach dem der PR approved ist, springt eh die Pipeline an und Tests laufen da u.a. auch durch.
Jedoch darf bei uns eh nur vorab gepusht werden, wenn alle Tests lokal bestanden sind. Also da gibt es selten Probleme.

 

[Zrebna]

Änderungen die nicht mehr lokal sind, sondern sich über mehrere Teile erstrecken checke ich lokal in meiner IDE aus. Ich schaue da auch online mir das Diff an, aber in einigen Stellen wechsel ich dann in die IDE um mehr Kontext zu haben und auch mal links und rechts zu schauen. Relevant ist da auch - was einem kein Tool abnehmen kann - die Erfahrung. Den interessant sind bei solchen Changes auch immer die Stellen, die nicht angepasst wurde, aber hätten angepasst werden müssen. Die findet man nur mit Erfahrung und einem funktionalen Test. Den Browser mit dem Online Diff brauche ich aber eh immer, weil da muss ich ja alle Anmerkungen reinschreiben wenn was nicht passt.

Ja, so habe ich mir das auch vorgestellt. Also eine Kombination zwischen dem Browser (muss ja eh, wegen Comments und weil man die Diffs sehen will) und für mehr Kontext dann in der IDE.

Ok, so werde ich es auf jeden Fall mal versuchen - Danke euch 🙂