phpBB Ext Check - Diskussion bezüglich Prozedur und Reports

[LukeWCS]

Das war glaube ich per PN, bin mir aber nicht sicher.

Dachte ich auch. Laut CL habe ich einen ersten Workaround für das Problem bei 1.0.0 eingebaut, das war im April 2020. Richtig gelöst habe ich die ganze Problematik dann erst bei 1.2.2. Aber in dem Zeitraum (2020/04) finde ich bei den PNs nichts zu dem Thema. Ist mir grad ein Rätsel, wo wir das besprochen haben.

edit: Habs. War per E-Mail.

[LukeWCS]

Der Startbeitrag berücksichtigt jetzt den neuen PHPCompatibility-Auto-Modus und wurde auch ansonsten etwas überarbeitet.

[LukeWCS]

Aufgrund eines aktuellen Falles, sprich zwischengespeicherte Berichte im EC Server, habe ich die Hinweise im Startbeitrag erweitert. Hier auch nochmal explizit: wenn EPV Warnungen (gelb) bezüglich "unbekannter" Dateitypen meldet, kann man das ignorieren, wenn der EPV Bericht sonst keine Fehler meldet.

Kann z.B. auftreten, wenn man aus Versehen eine .editorconfig im Ext Ordner platziert hat.

[LukeWCS]

2. Für die Download Extension werden auch über 120 Fehler zu fehlender Initialisierung von Array Variablen aufgelistet.
Genau diese hatte ich überall gesetzt und das wurde mir auf phpbb.com bei der Validierung der Extension auch prompt um die Ohren gehauen.
Begründung:
Man braucht kein Array initialisieren, wenn dieses vorher noch nicht verwendet / gesetzt ist, da die erste Zuweisung ein Array automatisch initiiert.
Die konkrete vorherige Initialisierung als "leeres" Array ist daher überflüssig und sollte somit unterbleiben.

Nachtrag: ich habe mir jetzt mal ein paar Codestellen angeschaut um das besser zu verstehen. Auch weil du geschrieben hast, dass der Validator eine Array Initialisierung kritisiert, was ich ehrlich gesagt ziemlich seltsam finde. Die Initialisierung von Variablen (auch Arrays) dient nicht nur der Fehler-Prävention, sondern auch der Lesbarkeit. Insbesondere wenn man in einer Funktion alle benötigten Variablen am Anfang definiert. Das ist auch etwas, woran ich mich bei PHP nur sehr schwer gewöhnen kann. Bei anderen Sprachen ist sowas sogar Pflicht, alleine schon weil der Compiler sonst den Source ganz einfach verweigert.

Ich habe mir gleich die erste Datei im Bericht angeschaut, also oxpus\dlext\controller\acp\acp_categories_controller.php und dabei die Variable $real_ver_file untersucht. Okay, das ist wirklich nicht trivial für VA, weil es sich hier nicht nur um ein simples Array, sondern gleich um ein assoziatives mehrdimensionales Array handelt. Das VA da schwindlig wird, kann ich verstehen. Sprich, dass es die Verwendung dieser Variable nicht wirklich erkennt. Denn so wie der Code geschrieben wurde, ist das kein Fehler. Nur VA blickt es nicht.

[oxpus]

Kann ich verstehen.
Ist ja auch nicht immer logisch, dass die Initalisierung fehlen darf.
Nun, verschieben wir das auf unbestimmte Zeit.
Vielleicht ergibt sich irgendwann mal etwas neues hierzu.

[LukeWCS]

Ist ja auch nicht immer logisch, dass die Initalisierung fehlen darf.

Eben.

Vielleicht ergibt sich irgendwann mal etwas neues hierzu.

Das ist ja auch nichts "exotisches" was du geschrieben hast. Ein mehrdimensionales assoziatives Array kriegt man ja zum Beispiel automatisch, wenn man einfach eine JSON Struktur von einer composer.json in ein Array lädt.

Ok, Payton Swick der Autor von VA ist ist offen für Vorschläge und Bug Reports. Er hat meinen Bug Report sehr schnell aufgegriffen und ruckzuck einen Fix bereitgestellt. Du könntest Payton das Problem direkt schildern, hast ja auch einen GH Account.

[LukeWCS]

Ich habe eine gezielte Meldung getriggert, weil das unmittelbar euch beide betrifft.

Chris, momentan hast du im Bericht für "Cookie Manager" knapp 400 Fehler die für dich gar nicht relevant sind. Es gäbe einen Weg das als Drittanbieter Komponente zu behandeln, was dann von EC ignoriert werden könnte.

Mike, bei "Usermap for phpbb" musst du ja - aus Validierungs-Gründen - die "ungepackte" Version von leaflet mitliefern. Ich hatte ja damals vendor vorgeschlagen, aber nach Stöbern auf .com zeichnet sich jetzt das Bild ab, dass das eher nicht so geschickt ist. In diesem Ordner sollen anscheinend nur Composer Pakete platziert werden. Siehe auch: https://www.phpbb.com/community/viewtopic.php?t=2288206

Ich arbeite seit ein paar Tagen an einem EC Update und dabei habe ich mir auch mal die CS Parameter genauer angeschaut und einiges probiert. Bei CS kann man nicht nur absolute Pfade von der Prüfung ausschliessen, sondern auch beliebige Ordner die sich auch an beliebigen Stellen innerhalb der Ordner-Hierarchie befinden können. Das heisst wir könnten folgendes machen, um die genannten Probleme zu lösen: einen Ordnernamen festlegen und in der Ext verwenden, der zum einen den Validatoren zeigt, das es sich hier um Drittanbieter Komponenten handelt und zum anderen in EC die Möglichkeit bietet, Ordner mit einem solchen Namen gezielt zu ignorieren.

Dadurch könnten beide genannten Probleme behoben werden. Ich würde als Ordnernamen thirdparty vorschlagen. Aber das kann man ja auch jederzeit ändern.

[Mike-on-Tour]

Das zitierte Thema scheinen auch die Validatoren auf .com nicht zu kennen, denn bei der ersten Validierung der Usermap wurde einiges beanstandet, aber nicht das Mitliefern der ungepackten leaflet-Datei im Ordner /vendor; jetzt bin ich mal gespannt auf das Ergebnis der zweiten Validierung.

Ansonsten finde ich den Vorschlag mit dem Ordner /thirdparty gut und würde das dann zukünftig auch so machen.

[LukeWCS]

Moin

Das zitierte Thema scheinen auch die Validatoren auf .com nicht zu kennen, denn bei der ersten Validierung der Usermap wurde einiges beanstandet, aber nicht das Mitliefern der ungepackten leaflet-Datei im Ordner /vendor; jetzt bin ich mal gespannt auf das Ergebnis der zweiten Validierung.

Jupp, wie ich schon gemerkt habe, hängt es davon ab, welchen Validator man hat. Bei einem Kollegen wurde etwas kritisiert, was bei mir ebenfalls dutzendfach vorhanden ist, aber nicht bemängelt wurde. Wobei es sich da auch eher um eine Empfehlung handelt so wie ich das sehe, denn auf das Ergebnis bezogen, macht die vorgeschlagene Änderung keinen Unterschied. Nur der technische Weg ist minimal anders.

Was solche Drittanbieter-Komponenten betrift, scheint es keinen Konsens zu geben, wie sowas gehandhabt werden soll/muss. Zum einen steht dazu nichts in den Dokus und zum anderen liest man auch die unterschiedlichsten Meinungen zu dem Thema. Sowas in einen thirdparty (Variante: 3rdparty) Ordner zu packen, taucht aber auf .com mehrmals auf, weshalb ich das so aufgegriffen habe. Und technisch ist es egal ob man ein JS noch in einen extra Ordner packt, man muss nur den zugehörigen INCLUDEJS entsprechend anpassen und gut ist.

Aber der extra Ordner sagt dem Validator eben, dass das eine fremde Komponente ist und PSSE kann solche Ordner dann gezielt ignorieren. Denn die phpBB Richtlinien gelten ja nur für uns, nicht für Autoren von Drittanbieter-Komponenten. Dementsprechend braucht PSSE das ja auch nicht auf phpBB Richtlinien zu prüfen, das produziert nur sehr viele irrelevante "Fehler".

Ansonsten finde ich den Vorschlag mit dem Ordner /thirdparty gut und würde das dann zukünftig auch so machen.

Habe ich gestern in EC auch schon eingebaut und wird dann erstmals bei 1.4.11 aktiv sein. Für dich im Prinzip völlig schmerzfrei, brauchst ja nur vendor umbenennen. Chris müsste minimal mehr machen.

[Mike-on-Tour]

Habe ich gestern in EC auch schon eingebaut und wird dann erstmals bei 1.4.11 aktiv sein.

Werde ich dann zukünftig berücksichtigen