RE: Commit-Regeln für die Firmware

Mo Apr 6 22:01:04 CEST 2020

Hallo,

zwei Wochen sind rum, und der einzige, der geantwortet hat, war Christian.

Da Christian im Wesentlichen die Meinung vertritt, es so zu belassen, wie es im Moment ist, und sonst keine Meinung vorhanden ist, ist die Sache damit für mich erledigt.

Beste Grüße

Adrian

> -----Original Message-----
> From: Christian Dresel [mailto:fff at chrisi01.de]
> Sent: Sonntag, 22. März 2020 21:53
> To: mail at adrianschmutzler.de; franken-dev at freifunk.net
> Subject: Re: Commit-Regeln für die Firmware
> 
> Hi
> 
> gleich vorweg, ich hab keine Erfahrung aus anderen Projekten und beziehe
> meine Aussagen von hörensagen und was ich in letzter Zeit persönlich
> einfach passend fand.
> 
> On 22.03.20 21:28, mail at adrianschmutzler.de wrote:
> > Hallo zusammen,
> >
> > schon seit einiger Zeit machen sich ja verschiedene Leute Gedanken, ob
> wir mit der relativ kleinen Zahl an Entwicklern ggf. etwas an den
> ungeschriebenen Regeln ändern sollten, wann ein Patch committed werden
> darf.
> >
> > Bisher habe ich den modus operandi so verstanden (und praktiziert), dass
> ein Patch nur dann applied wird, wenn mindestens ein Review vorhanden ist
> (ggf. auch mehr).
> > Dies stellt einerseits sicher, dass immer vier Augen einen Patch geprüft
> haben, führt auf der anderen Seite aber auch dazu, dass die Entwicklung
> gebremst wird und bei trivialen Patches im Prinzip auch Ressourcen
> verschwendet werden.
> 
> ich hab das mittlerweile so gehandhabt, das ich so Kleinigkeiten die auf der
> ML gelandet sind kurz angeguckt und dann einfach ein review dagegen
> geworfen habe. Siehe die 2 Sachen heute. Ich finde es irgendwie doch gut,
> wenn einfach noch ein 2 Paar Augen drüber guckt. Mir wurde immer wieder
> erklärt, ein Review ist im Prinzip nix anderes wie:
> "Ich hab mal über den Code geguckt, weiß zum größtenteil was er tut, er
> passt ins Konzept und es ist kein quatsch"
> Review heißt nicht:
> "Ich bin mir sicher das der Code absolut perfekt tut, hab ihn gebaut und
> geflasht und keinerlei Sicherheitslücken enthält und hab ihn komplett
> durchgetestet"
> 
> Das hab ich in letzter Zeit auch so gelebt und finde es ansich recht passend.
> So ein "ich hab da ein Komma hinzugefügt" ist zwar nur ne Kleinigkeit, da
> kann ich aber auch mal eben $Mod+P drücken mein Gruß Christian drunter
> klopfen und rausschicken, dauert keine Minute und es hat ein 2. Paar Augen
> einfach mal drauf geguckt. Sonst hat man so ne schwimmende Grenze:
> "Da ein Komma geht schon"
> "Da ein Satz geht schon"
> "Da eine Codezeile ist ja auch nicht viel"
> "Da die ganze Funktion auf den Kopf stellen ist doch nur ne Kleinigkeit"
> "..."
> also wo zieht man dann die Grenze?
> 
> >
> > Auf der anderen Seite habe ich bei OpenWrt ein System kennengelernt,
> dass dem einzelnen Committer deutlich mehr Macht verleiht. Im
> Wesentlichen entscheidet dort jeder selbst, ob er einen Patch direkt applied
> oder vorher zum Review an die Mailingliste schickt. Je nach Schwere des
> Eingriffs wartet man dann eine gewisse Zeit, und applied den Patch
> entweder wenn kein Veto kommt oder nach entsprechendem Acked-by
> oder Review.
> > Sowohl die Entscheidung, die Liste zu befragen, als auch die Wartezeit/das
> Kriterium für den Merge entscheidet jeder selbst. Wer bloß der Sortierung
> oder einen Tippfehler ändert, kann dies einfach reinwerfen, wer
> configurehood komplett umbauen will, würde wohl besser nachfragen. Ggf.
> reicht z.B. auch ein Acked-by aus, wenn man sich mit dem Code sicher ist,
> aber nicht weiß, ob das Feature gewünscht ist etc.
> 
> dieses acked-by hab ich eher so verwendet:
> "Commitmsg beschreibt etwas was ich haben will, den Code hab ich mir aber
> nicht weiter angeguckt"
> 
> Ich finde nicht, das es für ein applien ausreichen sollte.
> 
> Es gibt ja dann noch das tested-by, das heißt für mich dann, der Code ist
> komplett durchgetestet und funktioniert einwandfrei.
> 
> Ich würde also dahingehen das:
> Zum applien ein reviewed-by weiterhin nötig ist. Aber hier auch nochmal
> klarstellen das ein reviewed-by eben kein "Code komplett durchgetestet
> und tut" ist sondern das was ich oben bereits geschrieben habe.
> Im Gegenzug fände ich es gut, wenn sehr komplexe oder kritische umbauten
> (muss jeder für sich entscheiden was darunter fällt) vllt. sogar ein tested-by
> haben sollten. Das würde für mich mindestens heißen: "Ich hab es gebaut,
> geflasht und der Code tut augenscheinlich das was zu erwarten war" (kommt
> am Ende natürlich auf den Einzelfall an, bei einem Komma in der README ist
> es Käse, bei hochziehen von OpenWRT z.b. wäre es sinnvoll es zumindest für
> alle Geräte durchzubauen und auf ein Gerät zu flashen mMn.). Dieses
> tested-by kann ja auch der Patchersteller machen, ich denke wer ein
> komplexen Patch baut, wird ihn zumindest auch einmal testen oder? Wenn
> dann noch jemand ein reviewed-by (Bedingungen siehe
> oben) drüber wirft, wäre das ok zum applien.
> 
> Hätte z.b. der Patchersteller das OpenWRT 19.7 mal mit allen devices
> durchgebaut, wäre wohl aufgefallen, dass das cp vom ER-X(-SFP) failed (soll
> jetzt keine Anschuldigung sein bitte nicht übel nehmen aber es passt gerade
> gut, nur ein Beispiel was besser gewesen wäre wenn man meinen oberen
> Fall leben würde).
> 
> Oder anderes Beispiel:
> Wer ein neues Device himzufügt, wird es ja mindestens 1x testen. Auch hier
> kann der Ersteller des Patches dann eigentlich gleich ein tested-by dran
> hängen, ein Reviewer guckt kurz drüber ob er echt nur das device hinzufügt
> und nicht vllt. irgendwo versucht groben Unfug mit reinzuschmuggeln, macht
> ein reviewed-by und alles ist gut.
> 
> >
> > Ich möchte hier eine Diskussion anregen, ob für uns eine Öffnung zu
> entsprechenden Regeln mit mehr Macht und Flexibilität für den einzelnen
> Maintainer gewünscht ist.
> > In meinen Augen kann dies sinnvoll sein, sofern bestimmte Bedingungen
> erfüllt werden:
> >
> > 1. Bestimmte minimale Grundkriterien für einen akzeptablen Commit. Für
> mich wären das ein ganz knapper Text der Form von Commit
> Title/Message/xxx-by sowie sinnvolle Aufteilung von Commits irgendwo
> fixiert.
> >
> > 2. Ein klar definierter Personenkreis, der Commit-Rechte besitzt.
> > Zugriff auf das firmware-repo haben z.Zt:
> > RedDog
> > Steffen Pankratz kratz00
> > fblaese
> > meskal
> > mayosemmel
> > adrianschmutzler
> 
> hier würde ich anregen die Leute mal nochmal persönlich anzuschreiben
> (falls sie jetzt nicht schon hier antworten) und fragen ob sie das überhaupt
> noch machen möchten und wenn keine Antwort kommt runternehmen.
> 
> 
> >
> > 3. Bei allen Beteiligten wird ein verantwortungsvoller Umgang
> vorausgesetzt, Änderungen sollten entsprechend durchdacht und ggf.
> getestet sein. Je unsicherer, desto eher macht es Sinn, nachzufragen oder
> auf Reviews zu warten.
> 
> 4.
> Wie wollen wir es mit Klarnamenpflicht bei signed-off und reviewed-by/etc.
> halten? Die Diskussion kam ja auch schonmal auf. Ich bin hier sehr unsicher.
> 
> Gruß
> 
> Christian
> 
> >
> > Über Meinungen würde ich mich freuen.
> >
> > Beste Grüße
> >
> > Adrian
> >
-------------- nächster Teil --------------
Ein Dateianhang mit Binärdaten wurde abgetrennt...
Dateiname   : openpgp-digital-signature.asc
Dateityp    : application/pgp-signature
Dateigröße  : 834 bytes
Beschreibung: nicht verfügbar
URL         : <https://lists.freifunk.net/pipermail/franken-dev-freifunk.net/attachments/20200406/cc9a6b90/attachment.sig>