phpBB Ext Check - Diskussion bezüglich Prozedur und Reports

[Mike-on-Tour]

Wenn ich EPV richtig verstanden habe, kann man diesen Test mit 4 zusätzlichen Zeilen implementieren:
https://github.com/Mike-on-Tour/epv/blo ... p#L82..L86, neue Zeilen 82-86

Kannst du das vielleicht mal in einer Testumgebung von ExtCheck ausprobieren?

[chris1278]

Ich versteh nicht so viel von php code. Daher schreib ich einfach mal frei raus was ich denke.

Okay, grundsätzlich auf diesen Ordner bestehen kann man nicht. Denn es gibt ja auch Sprachneutrale Exts die gar keine zusätzlichen Sprachvariablen brauchen, weil sie entweder Bord-eigene nutzen, oder generell keine Meldungen/Anzeigen haben. Allerdings sollte man doch annehmen, dass EPV sich wenigstens dann meldet, wenn es mindestens ein nicht-englisches Paket findet, aber das englische Paket fehlt. Das ist aber nicht der Fall.

Hier wäre es sinnvoll wenn man es umsetzen könnte vielleicht diesen ansatz zu nehmen:

Erstmal prüft man ob überhaupt ein Verzeichniss namens language existiert und wenn das nicht existiert brauchts auch net zu unterscheiden oder prüfen ob eben ein verzeichniss en oder so da ist. Exisitiert dies dann wäre es hierbei sinnvoll auf das en Verzeichniss zu prüfen unabhängig ob der Autor die Extension auf phpbb.com einreicht oder eben nur z.B. hier im Deutschen Forum veröffentlicht.

Ich denke das könnte man ja so gestallten das es zwar nicht als grundsätzlichen Fehler ausgibt sondern einfach als Warnung mit info etwa so was ind er Art:

Es wurde ein ordner /language gefunden aber dort befindet sich kein ordner /en für die Englische Sprache. Bedenke das wenn du die Extension auf phpbb.com veröffenlichen willst ein solches Englisches Sprachpaket vorhanden sin muss.

Oder so was in der Art halt. Das nur mal so als gedanken Vorschlag meinerseits.

[Mike-on-Tour]

So wie ich diesen Code verstanden habe, werden diese Tests nur durchgeführt, wenn ein Verzeichnis language innerhalb der Erweiterung existiert.
Natürlich hast du Recht, dass dieser Test nur dann Sinn macht.

[chris1278]

Ja ich hab den Code nur eben mal überflogen. Dann ist das ja gut wenn er das so macht.

[LukeWCS]

@Chris

Jupp, deine Überlegung ist berechtigt. Auf en darf natürlich nur geprüft werden, wenn auch der Ordner language vorhanden ist, oder besser gesagt; wenn ein nicht-englisches Paket vorhanden ist.

Wenn ich EPV richtig verstanden habe, kann man diesen Test mit 4 zusätzlichen Zeilen implementieren:
https://github.com/Mike-on-Tour/epv/blo ... p#L82..L86, neue Zeilen 82-86

Kannst du das vielleicht mal in einer Testumgebung von ExtCheck ausprobieren?

Elegant gelöst.

Funktioniert grundsätzlich einwandfrei, aber tatsächlich wird deine Abfrage auch dann ausgeführt, wenn es den Sprachordner gar nicht gibt. Ich hätte wie du angenommen, dass dann die ganze Methode gar nicht ausgeführt wird. Das ist aber nicht der Fall, das wird trotzdem ausgeführt.

Okay, das kann man natürlich nur feststellen, wenn man auch eine lokale EPV Installation hat. Mit einer kleinen Änderung läuft dein Code wie beabsichtigt:

Code: Alles auswählen

if (!empty($langs) && !array_key_exists('en', $langs))

Achja, ich hatte mich zuerst gefragt, ob es gerechtfertigt ist gleich einen FATAL zu triggern, statt einem ERROR. Dann habe ich mal verschiedene Szenarien ausprobiert bezüglich Ext Sprachpakete und bin dann zur gleichen Einschätzung wie du gekommen. Ein FATAL ist definitiv gerechtfertigt: Wenn man Englisch einstellt und das englische Paket fehlen sollte, nimmt phpBB als fallback die Sprache, die bei "Standard-Sprache:" in den Board-Einstellungen definiert wurde. Wenn dieses Paket in der Ext aber auch fehlen sollte, wird ein PHP Fatal error ausgelöst und es geht gar nichts mehr. Weder im Frontend, noch im ACP. Bis eben wusste ich nicht, dass man phpBB schon durch sowas triviales "abschiessen" kann.

Danke Mike, wieder ein Stück besser. Ich werde heute Abend deine Änderungen direkt in mein Patchsystem von EPV einpflegen, damit diese Änderung automatisch bei einem Composer Update durchgeführt wird, sofern die Originaldatei von EPV beim Update nicht verändert wurde. Ich melde mich dann nochmal hier, wenn deine Änderung live ist.

edit: Mike, damit du das in Aktion siehst, hier ein Screen vom lokalen EPV. Die Farben sehen bei EC natürlich anders aus.
[ externes Bild ]

[Mike-on-Tour]

Danke für die Rückmeldung und das Bild, sieht gut aus.

Und ja, deine Lösung mit dem zusätzlichen !empty($langs) && im if-clause hätte ich dann auch so gemacht, wenn ich gemerkt hätte, dass dieser Test auch bei einem nicht vorhandenen Sprachverzeichnis durchgeführt wird (ich habe jetzt nochmal genauer nachgesehen und festgestellt, dass da tatsächlich keine entsprechende Vorabprüfung durchgeführt wird, es werden einfach stumpf alle vorhandenen Tests aufgerufen).

Ich habe mir den EPV geforkt und das Ganze als PR ausgeführt, dann muss ich das also nochmal abändern.

EDIT:
Dass man phpBB damit komplett abschießen kann, war mir schon klar, habe ich nämlich auch schon geschafft und bin nur durch das Error-Log auf die Ursache gekommen. Deshalb wundert es mich ja, dass EPV das nicht prüft.

[LukeWCS]

Deshalb wundert es mich ja, dass EPV das nicht prüft.

Ich hätte deinen Code sowieso eingebaut, alleine wegen Richtlinien. Aber jetzt erst recht, wo ich weiss was passieren kann wenn das en Paket fehlt.

[oxpus]

Nur um es mal kurz festzuhalten:
Wenn EPV schon nicht alles genau nach den selbst gesteckten Richtlinien prüft... was erwartet uns dann möglicherweise noch?
Man kann sich dann doch darauf gar nicht verlassen, wenn solche "Kleinigkeiten", auf die nach den Coding Guidelines dringend zu achten sind, nicht ordentlich geprüft werden.
Aber den Extension Autoren dann noch ganz andere "Banalitäten" vorwerfen, die zur Ablehnung führen.

Tztztz...

So, musste mal raus.

Zum Thema:
Danke an Euch für diese Ergänzungen.
Damit kann ich mich wieder mehr auf das verlassen, was ohnehin vorgegeben wurde und jetzt auch ordentlich geprüft wird.
Für mich ist und bleibt diese Testsuite definitiv unverzichtbar geworden, unabhängig der auf phpbb.om bereitgestellten EPV-Testsuite, GitHub Actions und den Prüfungen in meinem eigenen "hochgezüchteten" VS-Code-Editor.

[LukeWCS]

Man kann sich dann doch darauf gar nicht verlassen, wenn solche "Kleinigkeiten", auf die nach den Coding Guidelines dringend zu achten sind, nicht ordentlich geprüft werden.

Es gibt leider etliche Details die nicht geprüft werden. Wobei EC ja ohnehin schon deutlich umfangreicher prüft, als das offizielle Paket.

Aber den Extension Autoren dann noch ganz andere "Banalitäten" vorwerfen, die zur Ablehnung führen.

Bin da ganz bei dir. Es gibt so manche Ungereimtheiten, die ich in der Richtung in den letzten Jahren gesammelt habe. Da kann schon mal "leicht" Frust aufkommen.

Darum ist es gar nicht verkehrt bei .de was eigenes zu haben. Da haben wir dann auch mehr Kontrolle und können das eine oder andere selbst nachrüsten. Was auch schon mehrmals geschehen ist und zwar entweder direkt oder indirekt durch:

Kirk
chris1278
gn#36
Dr.Death
oxpus
Mike-on-Tour



Mikes Änderung ist jetzt live.

[Mike-on-Tour]

Gerade mal ausprobiert, sieht gut aus