Re: Commit-Regeln für die Firmware

[Christian Dresel]

Hi

Am 22. Juli 2021 14:01:14 MESZ schrieb Robert Langhammer <rlanghammer at web.de>:
>Hallo,
>
>wir müssen der Wahrheit ins Auge sehen! Es ist leider so, dass kaum
>mehr
>Entwicklerkapazitäten vorhanden sind. Die Gründe hierfür mögen
>vielfältig sein, und sollten an anderer Stelle auch diskutiert werden.

mMn sollte die komplette Community mit einbezogen werden. Ich denke du wolltest auf das gleiche hinaus?

Gruß

Christian

>
>Ich unterstütze den Vorschlag.
>
>Viele Grüße
>Robert
>
>
>Am 22.07.21 um 12:36 schrieb Fabian Bläse:
>> Hallo zusammen,
>>
>> ich möchte diese Idee noch einmal entstauben und neu bewerten.
>> Ich bin damals wohl irgendwie nicht dazu gekommen, auf diese Mail zu
>antworten. Möglicherweise hatte ich da grade viel um die Ohren.
>>
>> Aktuell ist das Problem wieder relativ akut, dass viele eigentlich
>wichtige Patches liegen bleiben, keine Reviewer finden, und
>entsprechend nicht applied werden können. Meine eigenen Änderungen kann
>ich selbst ja nicht reviewen.
>>
>> Das ganze bremst die Entwicklung aktuell ziemlich, obwohl ich
>eigentlich gerne möglichst zügig auf ein DSA Release (oder zumindest
>beta) zusteuern möchte, da die EdgeRouter X im aktuellen Zustand mit
>dem bekannten Ethernet Bug kaum zu gebrauchen sind.
>> Ebenso wird auch das Performanceproblem von babeld immer
>problematischer, speziell auf langsameren Routern.
>>
>> Wenn jeder Entwicklungsschritt erst wochenlang herumliegt, und dann
>am Ende mangels Review doch nicht applied werden kann, macht das
>Firmware-Frickeln auch nur sehr eingeschränkt Spaß.
>> Vielleicht müssen wir unsere doch recht hohen Qualitätsstandards
>(ein, am besten zwei Reviews für jede(!) Änderung, ..) ein wenig
>zurückschrauben, da wir dafür scheinbar nicht ausreichend
>Entwicklerkapazität (oder Interesse?) haben. Von einer Firmware, deren
>Entwicklung stagniert, hat auch niemand etwas.
>>
>> Daher möchte ich die Idee von Adrian noch einmal Vorschlagen:
>>> 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.
>>>
>>> 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.
>> Ich wäre sehr dafür den aktiven Entwicklern entsprechende Rechte
>einzuräumen, so dass die Entwicklung - speziell auch von einfacheren
>(aufräum-) Patches - beschleunigt werden kann. Besonders dann, wenn
>gerade nur wenig Review-Kapazität verfügbar ist.
>>
>> Gruß
>> Fabian
>>
>>
>> On 06.04.20 22:01, mail at adrianschmutzler.de wrote:
>>> 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
>>>>>

-- 
Diese Nachricht wurde von meinem Android-Gerät mit K-9 Mail gesendet.