[3.3] Recent Topics NG

[chris1278]

Schön das ich helfen konnte.

[IMC]

'n Abend Patrick,

ich habe noch ein paar unnötige Kommentare entfernt. Damit sollte RTNG aus meiner Sicht fertig sein.

Wenn du den Code gesichtet hast sollten wir entscheiden ob wir RTNG auf Final oder RC2 setzen. Danach werde ich den Changelog anpassen und die Branches zusammenführen.

[LukeWCS]

Habe mir den aktuellen Stand von dev geholt und grob gesichtet. Dabei paar Typos hier und da behoben. Dann hatte ich auch das mit "Wer is online" getestet. Das funktioniert zwar, triggert aber ein NOTICE, wenn ein User auf einer der separaten RTNG Seiten ist:

Code: Alles auswählen

[phpBB Debug] PHP Notice: in file [ROOT]/ext/imcger/recenttopicsng/event/main_listener.php on line 107: Only variables should be passed by reference

Ursache ist end(). Der passende Doku Abschnitt:

Das Array. Das Array wird per Referenz übergeben, weil es von der Funktion bearbeitet wird. Das bedeutet, dass der Funktion eine echte Variable übergeben werden muss und nicht beispielsweise der Rückgabewert einer Funktion.

Anders ausgedrückt: die Funktion liefert nicht nur das letzte Array Element, sondern manipuliert auch den Array Pointer. Letzteres kann aber nur funktionieren, wenn das Array auch als dediziertes Datenobjekt (sprich Variable) vorhanden ist. Direkt auf eine Funktions-Rückgabe anwenden geht nicht.

Hab ich auch gleich behoben. Und ist vermutlich eh nur eine Frage der Zeit, bis das nicht mehr nur als NOTICE gemeldet wird, da PHP stetig strenger wird.

Eine Alternative wäre:

Code: Alles auswählen

$session_page = explode('/', $event['row']['session_page']);
$site = $session_page[array_key_last($session_page)];

Dabei wird der Pointer nicht verändert. Finde ich aber umständlich, weil in der zweiten Zeile die Array Variable zweimal notiert werden muss. end() ist eleganter und der Pointer kann uns dabei auch nicht in die Quere kommen, weil wir ihn gar nicht brauchen.

Es gibt noch etliche weitere Varianten, aber die sind teilweise echt abenteuerlich und lesen sich dann so, als ob man versucht hätte, den Code möglichst schwer lesbar zu machen.

Was Release angeht, würde ich sagen wir halten uns nicht mehr mit rc2 auf, da die Rückmeldungen schon zu rc1 "überschaubar" waren. Ich mach heute Abend nochmal ein Update mit dem hier Besprochenen und dann kannst du dich ans Release machen.

Dann noch was anderes: wenn von deiner Seite nichts dagegen spricht, würde ich auch bei RNTG das offizielle phpBB GitHub Actions Framework einrichten. Wir als EC User sind darauf zwar nicht angewiesen, aber ich habe das trotzdem bei allen Exts eingerichtet, quasi als zusätzliche und unabhängige Prüfung. Wenn das einmal eingerichtet ist, läuft das für uns auch transparent, da müssen wir uns also nicht mehr drum kümmern und es behindert uns auch nicht. Und man kann auch gleich die bei vielen phpBB Ext Repos bekannte "Tests passing/Tests failed" Plakette anzeigen lassen bezüglich letztem Status der Tests. Falls OK, würde ich das gleich mit dazu packen.

[IMC]

Das funktioniert zwar, triggert aber ein NOTICE, ... Dabei wird der Pointer nicht verändert.

Ich habe das bei mir nochmal getestet um zu sehen ob mir da etwas durch gegangen ist. Einstellung in meine php.ini E_ALL, die php Version 8.4.7. Ich habe keine Meldung im Error-Log-File. Ein guter Anlass mal meinen Einstellungen zu testen. Damit ich die Notize auch bekomme.
Deinen Änderungsvorschlag finde ich gut.

Ich mach heute Abend nochmal ein Update mit dem hier Besprochenen und dann kannst du dich ans Release machen.

Mache ich fertig wenn du alles hochgeladen hast.

würde ich auch bei RNTG das offizielle phpBB GitHub Actions Framework einrichten

Finde ich gut. Ich habe mich damit noch nicht im Detail auseinander gesetzt. Ein guter Anlass mir das dann genauer anzuschauen.

[LukeWCS]

Einstellung in meine php.ini E_ALL, die php Version 8.4.7. Ich habe keine Meldung im Error-Log-File.

Im Log habe ich das auch nicht, sondern als direkte Anzeige in phpBB. Dazu muss aber der Debug Modus von phpBB aktiv sein, da sonst NOTICE Meldungen unterdrückt werden.

Deinen Änderungsvorschlag finde ich gut.

Falls du damit den Code Block meinst aus meinem vorletzten Beitrag; das ist nur eine Alternative zu meiner tatsächlichen Lösung die ich erst noch hochladen muss. Die entspricht dann genau deinem Code, nur minimal anders notiert.

Finde ich gut. Ich habe mich damit noch nicht im Detail auseinander gesetzt. Ein guter Anlass mir das dann genauer anzuschauen.

Das Framework in der zweiten Generation ist sehr einfach einzubauen. Und schon die erste Generation war erheblich einfacher einzubauen als noch das ganz alte System "Travis CI". Das war vielleicht ein Krampf, weil man da z.B. auch in der Shell erst ein Skript startfähig machen musste und ähnliche Scherze. Das war wirklich nur was für Nerds und alles andere als Praxis-orientiert.

Was wo wie bezüglich Test Framework gemacht werden muss, siehst du dann wenn ich die Änderungen nach dev gepusht habe.

[IMC]

Die entspricht dann genau deinem Code, nur minimal anders notiert.

Das ist noch besser. Ist wohl auch nur eine Zeile mehr. Das werde ich dann sehen.

Ich werde dann mal meine Debug Modus aktivieren um die Meldung nachzustellen.

[LukeWCS]

Updates gepusht. Ich habe das auf 2 Commits aufgeteilt; im ersten die GH Actions Einrichtung, im zweiten die hier besprochenen Änderungen. So ist das bisserl getrennt und du kannst problemlos erkennen was zu GH Actions gehört.

[IMC]

GH Actions sieht nicht so kompliziert aus wie ich dachte. Da hat sich einiges verbessert. Wie du auch schon schriebst.

Die NOTICE Meldung konnte nachstellen und in einer weiteren Extension beseitigen.

Den Rest mache ich morgen.

[LukeWCS]

GH Actions sieht nicht so kompliziert aus wie ich dachte. Da hat sich einiges verbessert. Wie du auch schon schriebst.

Jupp, MattF hat das deutlich abgespeckt was von unserer Seite vorhanden sein muss. Die erste Generation war da noch erheblich unübersichtlicher mit mehr Stolperfallen.

Damit du direkt mal was sehen kannst, war ich so frei und habe im "master" Branch GH Actions ebenfalls eingerichtet. So wird jetzt auch in beiden Branches in der README schon mal die Plakette angezeigt. Die bezieht sich übrigens in beiden Branches immer nur auf "master", also nicht wundern.

Im "Dev" Branch hab ich nochmal ein Update für die tests.yml gepusht, ich hatte da noch eine ältere Fassung. MattF hatte das zwischenzeitlich aber noch weiter entschlackt und zusätzliche Kommentare eingebaut. Diese aktuelle Fassung habe ich zwar schon hier im parallelen GH Actions Thema, aber die habe ich noch nicht bei meinen Exts und von EMP hatte ich das für RTNG kopiert. In beiden Branches von RTNG ist jetzt aber die aktuellste Fassung.

Was mich allerdings wundert; in "master" hätte der Job schon direkt ausgeführt werden müssen. Guck mal bitte in den Repo Settings unter "Actions" ob das deaktiviert ist. Wenn ja, bitte mal aktivieren.

Die NOTICE Meldung konnte nachstellen und in einer weiteren Extension beseitigen.

Alles klar.

[IMC]

Recent Topics NG 1.0.0 steht zum Download bereit.
Link im Startbeitrag