Pull request übernehmen: Bedingungen

[cpdef]

Ich denke wir sollten jetzt mal festlegen, wie wir darüber entscheiden, welcher Pull request übernommen wird:

marrin schrieb:

Also beispielsweise, dass einer die Rolle hat zu mergen, und ein Arbeitsablauf daraus besteht, dass zum Beispiel mindestens zwei Leute den Pull-Request für gut befunden und keiner einen Einwand hat, bevor gemerged werden darf. Sonst muss diskutiert und/oder nachgebessert werden. Eventuell macht eine Mindestzeit Sinn, damit jeder mal die Chance hatte in den Request-Inhalt rein zu schauen. Eventuell auch herunterladen und ausprobieren kann, wenn es sich um Code handelt.

Ich hätte so folgende Idee:

• Laufzeit 3 Tage
• es müssen mindestens 2 zustimmen
• mindestens einer aus dem Team QA muss zustimmen.

Das sind jetzt auch nur Vorschläge, also die Zahlen können durchaus noch geändert werden.

Das könnten wir dann auch so für andere Entscheidungen übernehmen, wie neue Teams etc. .

Die Zustimmung könnte man zum Beispiel durch dieses +1 Zeichen zum Ausdruck bringen. (Ich meinte zu einem Pull request und eigentlich nicht diesem Issue :) )

[sebastian2443]

:+1:

[psython]

Wie wäre es, wenn wir diese Regelung im Wiki vermerken?

Pull request bestätigen

Damit Verbesserungen im Master-Branch übernommen werde, müssen bestimmte Vorraussetzungen erfüllt sein.

1. Es muss immer mindestens ein Qualitätsprüfer die Änderungen genehmigen. 2.1. Das Team bekommt die Gelegenheit innerhalb von 3 Tagen auf ein pull request zu reagieren. Mit dem Emoji (:+1:) kann jedes Mitglied zeigen, dass die Anfrage positiv zur Kenntnis genommen wurde. Sobald die Mehrheit des Teams sich für die Anfrage aussprechen, ist das Mergen genehmigt. 2.2. Wird die Mehrheit innerhalb der 3 Tage nicht erreicht, erhalten die Qualitätsprüfer eine zusätzliche Stimme.

[cpdef]

Das mit der Mehrheit ist aber irgendwann nicht mehr umsetztbar, da es entweder zu viele Pull requests werden, wo jeder abstimmen muss, oder zu Viele Mitglieder, so dass immer viele abstimmen müssen.

Ich denke eine Regel mit einer bestimmten Anzahl Personen wäre geeignet. (z.B. 2 oder 4 müssen zustimmen)

Ich habe erst mal ein Artikel im generellen wiki erstellt.

[Dav1dde]

Wie wärs mit 2 müssen mindestens zustimmen, Laufzeit 2-3 Tage, keine Widersrpüche.

[cpdef]

also wenn jemand einen Widerspruch hat, müssen mehr Zustimmen oder wie?

[Dav1dde]

Nein, dann wird der PR geblockt bis eine Lösung gefunden wurde und kein Widerspruch mehr herrscht.

[cpdef]

Ok übernehme ich so erst mal, bis jemand Widersprüche hat :)

[seblin]

Ich bin der Meinung, dass mindestens einer aus dem QA-Team zustimmen sollte. Übrigens kann dieses Team gerne noch erweitert werden. Bis zu 5 Personen für QA fände ich schon ganz gut. Zumal sicherlich jeder woanders seinen Stärken und Schwächen hat.

[Dav1dde]

Find ich gut! Mindestens 2 und mindestens 1x QA? Ich frage mal an ob ich in die QA Gruppe darf ;)

[cpdef]

Die regeln stehen jetzt hier schon im wiki.

@seblin das hatten wir schon vorher besprochen :)

Können QAs denn auch welche werden, die sich noch nicht ganz so gut auskennen?

[seblin]

Denkbar wäre auch, die Rolle des QAs im Rotationsprinzip zu vergeben, sodass jeder mal sehen kann, wie es sich in dieser Rolle "anfühlt". Zum Beispiel immer 2 QAs, die monatlich wechseln. Oder erster und zweiter QA, wobei monatlich der erste QA seinen Dienst beendet, der zweite QA dann an die erste Stelle rückt und für den zweiten QA jemand neues nachrückt. Ich bin da offen für alles. Jeder sollte etwas von diesem Projekt für sich mitnehmen können und bei den besseren Programmierern sollten hier keine falschen Eitelkeiten entstehen.

[cpdef]

Ist der QA Plan so gut? Eigentlich können wir dann doch auch das Team entfernen oder?

[Dav1dde]

Brauchen wir überhaupt QA mit der "Kein Widerspruch" und mindestens 2 Votes Regel? QA-Rolle sollte ja nicht heißen, nur die Leute in QA machen QA.

[cpdef]

ok nehme ich raus. (Du meinst sicher die letzte Zeile oder?)

[psython]

Also es scheint so, als bräuchten wir keine Teams und keine Planung oder sonst was. Wir zögern nur das Projekt hinaus. Wie wäre es, wenn wir einfach anfangen zu coden und die Planung und sonstiges by the way machen?

[psython]

@cpdef kennst du die Vorschau Funktion im Forum? Die gibt es hier auch. Dann kannst du die Änderungen, die du vornimmst vor dem Absenden überprüfen. Wenn du zufrieden bist schickst du die Änderung raus. Das macht es leichter Veränderungen rückgängig zu machen und unsere Pinnwände bleiben übersichtlich.

[cpdef]

Werde ich in Zukunft beachten (wo habe ich da was falsch gemacht? url?). Das ohne Teams denke ich ist ok, falls es später Probleme gibt, können wir uns ja auch nochmal um entscheiden.

Am besten stimmen wir kurz mit :+1: ab ob wir die Teams löschen. Ich kann das wahrschenlich aber nicht mehr machen, da ich Morgen nur kurz und danach gar nicht mehr online bin.

Also :+1: = Teams werden gelöscht, :-1: eben nicht.

Ich habe die Nachricht auch mal in #3 gesendet, weil sie da eigentlich hingehört. Am besten machen wir da auch die Abstimmung.

[miracle173]

1. In der Wiki steht "Sobald 2 Mitglieder des Teams sich für die Anfrage aussprechen, gibt es genügend stimmen um die Anfrage zu genehmigen." Wa hat das für einen Sinn. Reicht es nicht, wenn keiner einen Einwand hat und der QP (Qualitätsprüfer) das für ok findet? Warum muss wer dafür stimmen? Abgesehen davon ist mir völlig unklar, wie das gemeint ist? Heisst das, dass sobald zwei dafür stimmen, der QP den Request sofort genehmigen kann?
2. @seblin hat oben die Idee gehabt, das man immer für 2 Monate QP ist, und und dass man das erste Monat mit einem QP vom Vormonat gemeinsam macht, eine Idee, die ich sehr gut finde, da ja irgenwie das Wissen übergeben werden muss. Beim derzeitigen Plan ist das nicht der Fall. Ich denke, dass sollte man ändern, oder seht ihr das nicht anders?
3. Ich stehe derzeit leider nicht zur Verfügung und habe mich aus der QP Liste entfernt.

[Marrin]

Ad 1. Das keiner einen Einwand hat reicht nicht, weil dann Sachen durchrutschen können die sich keiner angeschaut hat. Denn das kann kein Einwand ja auch bedeuten, das keiner Zeit oder Lust hatte sich das anzuschauen und es dann effektiv kein Review gibt.

QP sind keine besonderen Leute. Du bist ja auch dafür das diese Rolle rotiert, also werden das auch mal Anfänger. Ich denke da sollte die Stimme von der QP-Rolle nicht mehr Gewicht haben als andere.

[sebastian2443]

Ich finde auch, dass mindestens 2 Leute einer Änderung zustimmen sollten. Gerade für Anfänger und Fortgeschrittene ist das doch interessant, weil sie diesen Code testen und verstehen müssen. Dabei kommen Fragen auf und man tauscht sich aus. Aus soetwas entsteht dann viel besserer Code. Zumindest ist die Theorie so.

[miracle173]

Ich bin nicht vertraut mit dem ganzen. Ich habe mir gedacht, der QP wirft irgend ein Script an , das die komplette Programm mehr oder weniger testet, oder wenn solche Tests schon durchgeführt werden, überprüft er, ob sie erfolgreich waren. Möglicherweise überpürft er mit einer Liste noch einige andere Dinge. Und wenn das alles ok ist merged er das. Offensichtlich ist das ja nicht so. Was erwartet man, dass er tut und wa erwartet man bon denen, die zugestimmt haben, dass sie getan haben?

[Dav1dde]

Tests werden generell immer von einem selber ausgeführt bevor man auf den Remote/Master pusht bzw. von Jenkins, Sonar etc.

Die Reviewer (die dem PR zustimmen), gehen Zeile für Zeile den Code durch und schauen ob man etwas evt. schöner machen kann oder ob es logische Fehler gibt und ganz allgemein ob das überhaupt Sinn macht was dort geändert/eingeführt wird.

Code-Review ist etwas sehr wichtig und hilfreich aber halt auch sehr aufwendig.

[miracle173]

1. Der Entwickler führt einen Test seines Moduls durch. Er führt auch einen Integrationstest aus bzw einen Systemtest?
2. Die Tests müssen natürlich mitgepflegt werden und auch im github zur Verfügung stehen. Ist das richtig?
3. Das heißt aber, wenn jemand (möglicherweise nicht aus dem inneren Kern der Programmiergruppe), der ein paar Bugs im Code behebt, auch alle Tests durchführen muss. Der könnte aber Jenkins oder Sonar gar nicht installiert haben, ist das nicht eine ziemliche Hürde?
4. Kennst du dich mit Jenkin bzw. Sonar aus? Das schaut schon etwas komplex aus, Wollen/können wir das wirklich verwenden? Vielleicht kann man nur Pylint verwenden. Es Schaut aus als ob Sonar daruaf aufbaut.
5. Code-Review schaut etwas aufwendig auf. Und es sind nur 3 Tage Zeit. Im Allgemeinen ist auch der Entwickler dran beteiligt. Ich Allerdings könnte man versuchen, Code auf Stackexchange zu reviewen lassen. Das ist aber auf English. Was soll der QP eigentlich tun, wenn keine Zustimmung kommt. Er hat dann zwei Stimmen und kann also sein OK geben, wenn ich das richtig verstanden habe. Aber heisst das, er soll das auch selbst reviewen?

[Dav1dde]

Der Entwickler führt einen Test seines Moduls durch. Er führt auch einen Integrationstest aus bzw einen Systemtest?

Je mehr desto besser, im besten Fall läuft irgendwo sowieso ein Jenkins

Die Tests müssen natürlich mitgepflegt werden und auch im github zur Verfügung stehen. Ist das richtig?

Ja. Wenn mans richtig machen will Test Driven Development, d.h. Tests vor Implementierung.

Das heißt aber, wenn jemand (möglicherweise nicht aus dem inneren Kern der Programmiergruppe), der ein paar Bugs im Code behebt, auch alle Tests durchführen muss. Der könnte aber Jenkins oder Sonar gar nicht installiert haben, ist das nicht eine ziemliche Hürde? Jenkins/Sonar läuft auf mehreren Servern, wobei es eine Master-Instanz gibt die Builds/Tests koordiniert.

Kennst du dich mit Jenkin bzw. Sonar aus? Das schaut schon etwas komplex aus, Wollen/können wir das wirklich verwenden? Vielleicht kann man nur Pylint verwenden. Es Schaut aus als ob Sonar daruaf aufbaut.

Jenkins/SonarQube sind externe Programme, haben mit dem Projekt selbst nicht viel zu tun, so eine Instanz aufzusetzen ist jetzt nicht so wild, hätte eventuell eine Instanz die man mitbenutzen konnte.

Code-Review schaut etwas aufwendig auf. Und es sind nur 3 Tage Zeit. Im Allgemeinen ist auch der Entwickler dran beteiligt. Ich Allerdings könnte man versuchen, Code auf Stackexchange zu reviewen lassen. Das ist aber auf English. Was soll der QP eigentlich tun, wenn keine Zustimmung kommt. Er hat dann zwei Stimmen und kann also sein OK geben, wenn ich das richtig verstanden habe. Aber heisst das, er soll das auch selbst reviewen?

Warum 3 Tage (hab ich da was überlesen?)? 3 Tage ist tatsächlich wenig. StackExchange external Review, ich wusste nicht dass die da sowas machen, denke aber nicht, dass das eine gute Lösung ist. QA muss natürlich selber reviewen bevor er voted, mMn macht auch eine Rotation der Rolle nicht viel Sinn, entweder man packt Experten in die QA und gibt denen 2 Stimmen oder man lässt die QA Gruppe gleich ganz.

[Marrin]

Was die 3 Tage angeht: Wenn das zu wenig ist, stelle ich mir die Frage wie gross Ihr so einen Pull Request werden lassen wollt? Wenn das ein grosses Feature ist, dann braucht man vielleicht mehr Zeit, aber so wirklich gross sollte man Änderungen nicht werden lassen IMHO. Gerade bei dem Projekt hier, wo die Richtung nicht so ganz klar ist, und doch einige Anfänger dabei sind, wäre es blöd wenn jemand viel Arbeit in etwas steckt was den anderen dann in eine falsche Richtung abdriftet.

[seblin]

Und auch große Features sollte man möglichst in mehrere Commits splitten. Wenn die Implementierung eines neuen Features z.B. Änderungen an der bisherigen API voraussetzt, ist es manchmal sinnvoller, zuerst die API-Änderungen zu committen und ausgiebig zu testen und erst danach den Commit des neues Features einzubringen. Kommt natürlich immer auf den Einzelfall an.