Hooks im ucp_main.php

[tas2580]

Ich hab das zwar noch nie probiert, aber wenn ich das im Code richtig sehe ganz einfach mit

Code: Alles auswählen

$vars = array('foo', 'bar');
extract($phpbb_dispatcher->trigger_event('core.mein_event', compact($vars)));

Ist nur fraglich ob das Sinnvoll ist, denn beim nächsten phpBB Update funktioniert deine Extension nicht mehr.

Gruß Tobi

[D@ve]

Ist nur fraglich ob das Sinnvoll ist, denn beim nächsten phpBB Update funktioniert deine Extension nicht mehr.

Naja, wenn meine Vorschlag im Bugtracker bestätigt wurde, ist es eigentlich nur noch eine Frage der Benennung...

Ich hab das zwar noch nie probiert, aber wenn ich das im Code richtig sehe ganz einfach mit

Das kann ich mir eigentlich nicht vorstellen, dass es so einfach ist...

[Elsensee]

Für ein Event brauchst du natürlich den Namen.
Der fängt mit core. an und beinhaltet dann meist den Namen der Funktion beziehungsweise bei Modulen den Namen der Datei. Dann irgendwie die Stelle, aber das weißt du sicherlich schon.
Dann musst du gucken, welche Variablen sinnvoll wären. Also welche kann man gut gebrauchen und bei welchen macht es Sinn, diese zu verändern? Ich bin ein Inklusionist und würde daher eher immer zu viele Variablen übergeben als zu wenige...

Und dann fehlt bloß noch ein fetter Kommentarblock davor, wie du ihn sicherlich schon gesehen hast. Dieser wird erzwungen. Gibt es ihn nicht, werden die Tests auf GitHub fehlschlagen und dein Event gelangt nicht in den Core.

Ich hab das zwar noch nie probiert, aber wenn ich das im Code richtig sehe ganz einfach mit

Das kann ich mir eigentlich nicht vorstellen, dass es so einfach ist...

Doch. Zur Abwechslung ist es mal wirklich so einfach.
Das Array muss soweit ich weiß zwar immer $vars heißen, aber dann ist es lediglich diese Zeile danach: extract($phpbb_dispatcher->trigger_event('core.mein_event', compact($vars))); und der ganze Dokumentationsblock davor.
Mehr braucht es nicht für ein Event mit Variablen. Zu beachten ist jedoch, dass ab 7 oder 8 übergebenen Variablen das Array mehrzeilig geschrieben werden muss, also pro Zeile eine Variable. Auch das wird durch die Tests erzwungen.
Ein Event ohne zu übergebende Variablen geht natürlich übrigens auch und sieht nur geringfügig anders aus, aber ich denke nicht, dass das hier für irgendjemanden von Bedeutung sein wird.

Wenn die Tests dann alle erfolgreich sind werden Events in der Regel einfach so durchgewunken. Natürlich gucken die da noch, ob das sinnvoll ist oder dadurch Sicherheitslücken entstehen - das Übliche halt. Aber sonst drücken die quasi einfach auf "Merge pull request" und das Thema ist durch.

[D@ve]

Aber sonst drücken die quasi einfach auf "Merge pull request" und das Thema ist durch.

ok, hast mir gerade einen Grund gegeben mich endlich mal mit Github auseinanderzusetzen... Kriegt man das dort per Mail irgendwie mit, wenn ein Pull-Request akzeptiert wurde?

Gruß, Dave

[Elsensee]

Ich weiß nicht mehr, ob man das Einstellen musste, aber ich glaube das ist bereits standardmäßig so eingestellt, dass man E-Mails bekommt, falls sich was in den Issues bzw. Pull Requests tut, in denen man mitgewirkt hat - also auch beim Mergen.

Und Git ist echt nicht so schwer. Beachte aber das, was im phpBB Wiki steht.

[gn#36]

Standardmäßig wird man ziemlich mit Mails zu eigenen Pull Requests zugespammt Hinweise zu allen Änderungen bei eigenen Pullrequests sind dabei.

[D@ve]

Hab da mal ein Pull request gemacht:
https://github.com/phpbb/phpbb/pull/3288

Kann da mal jemand schauen, ob ich das korrekt gemacht hab?
Wenn ich das richtig verstanden habe, muss man ja vorher immer ein Ticket dazu aufmachen. Hab da jetzt das Ticket verlinkt, das eh schon erstellt hatte.

Hab gesehen, dass Area51 wieder auf ist... Was ist denn da jetzt besser? Das dort wie gehabt posten, oder ein Pull Request?

Gruß, Dave

[gn#36]

Hi Dave,

da sind ein paar formale Fehler drin:

• Der Pull Request soll bei dir nicht aus einem nichtssagenden Branch stammen, sondern der Branchname soll angepasst sein an den PR. Also bei Issues aus dem Tracker z.B. issues/[nummer] (ob das zwingend so sein muss bin ich mir nicht sicher)
• Die Commit Message besteht aus Überschrift <= 80 Zeichen, Leerzeile, Sonstigem Inhalt (optional), Ticketkennzeichen aus dem Tracker von .com
• Du musst das zugehörige Ticket in den Commit Messages in der letzten Zeile mit angeben (es kann sein, dass der jeweils letzte Commit reicht)
• Branchname gehört auch in die Commitmessage

Außerdem schlägt der Travis CI Test fehl: https://travis-ci.org/phpbb/phpbb/builds/46689539

1) phpbb_event_export_php_test::test_crawl_php_file with data set #0 ('viewtopic.php')
LogicException: $vars array does not match the list of '@var' tags for event 'core.viewtopic_assign_template_vars' in file 'viewtopic.php:735'

Der verrät dir auch, dass es deine Commit Message nicht mag:

Inspecting commit message of commit 1c8f74f9f35acb384966ff4056ea8c7568dbdb8f
Error: heading should be a sentence beginning with a letter.
You entered:
Added an event to viewtopic.php to modify template vars.
Syntax error on line 1:
>> Added an event to viewtopic.php to modify template vars.
Expecting: comment, header,
Aborting commit.

Schau dir z.B. mal Elsensees Pull Request an: https://github.com/phpbb/phpbb/pull/3283

Der Link in deinem Pullrequest funktioniert übrigens nicht.