phpBB Ext Check - Diskussion bezüglich Prozedur und Reports

[LukeWCS]

@Mike

Mit reichlich Verspätung, sorry. In den letzten 3 Wochen haben mich die ganzen Wenns und Abers bei ExtOnOff auf Trab gehalten.

Wie erwartet, perfekt integriert Mike.

Als Rückmeldung für dich: zwei Kleinigkeiten habe ich geändert.

1. Bedingung für die Beendigung der Slug-Behandlung:

Code: Alles auswählen

		if ($yaml_ary[$i] == '' || preg_match('/^\w+:/', $yaml_ary[$i]))

Dabei würde folgender Fall (weitere Eigenschaft mit untergeordneten Eigenschaften) nicht berücksichtigt:

Code: Alles auswählen

mot_tzv_index:
    path: /mot_tzv/{link}
    defaults: { _controller: mot.tzv.controller.mot_tzv_main:index, link: NULL }
    requirements:
        link: \+
        id:\d # error 16
    test:
        testa: 123
        testb: 456
    prefix: /feed

Er würde fälschlicherweise test: kritisieren:

42 | ERROR | "test:" must be followed by a space.

RegEx wie folgt geändert:

Code: Alles auswählen

		if ($yaml_ary[$i] == '' || preg_match('/^\w+\s*:|^ {4}\w+\s*:/', $yaml_ary[$i]))

2. Bedingung für den Start der Slug-Behandlung:

Code: Alles auswählen

		
		if (preg_match('/\s+requirements:/', $yaml_ary[$i]))

Dabei würde er einen eingerückten Doppelpunkt nicht erkennen und infolgedessen einen Fehler beim Abschnitt "requirements" komplett übergehen:

Code: Alles auswählen

mot_tzv_index:
    path: /mot_tzv/{link}
    defaults: { _controller: mot.tzv.controller.mot_tzv_main:index, link: NULL }
    requirements :
        link: \+
        id:\d # error 16
    test:
        testa: 123
        testb: 456
    prefix: /feed

RegEx wie folgt geändert:

Code: Alles auswählen

		if (preg_match('/\s+requirements\s*:/', $yaml_ary[$i]))

Werde die neue version von phpbb33yamlcheck heute Abend in EC integrieren. Danke dir! Wieder ein Stück besser.

[Mike-on-Tour]

An die Möglichkeit dass nach den Slugs noch andere Definitionen kommen könnten, hatte ich jetzt nicht gedacht weil die bei mir immer zum Schluss kommen, deshalb hatte ich das nicht berücksichtigt und bei dem eingerückten Doppelpunkt hast du natürlich auch Recht

Ansonsten ist keine Entschuldigung nötig, wir sind ja nicht bei einem Rennen - und ich habe auch erst die ver 0.5.0 von TZV fertig gestellt bevor ich mich an den yamlcheck gesetzt habe.

Und ansonsten gilt: Stillstand ist Rückschritt

[LukeWCS]

phpbb33yamlcheck Update 1.0.6 ist online.

[LukeWCS]

Seit dem letzten EC Update gab es mehrere Updates bei PHPC 10 und VA die ich heute übernommen habe. Ausserdem bekam EC auch ein kleines Update: Irrelevante PHPC Module werden jetzt gar nicht mehr im Bericht aufgeführt, wenn sie durch den PHPC-Auto-Modus übersprungen werden.

[LukeWCS]

ping

Mike, mal wieder was aus der Rubrik "Buschtrommel". Du hast ja bei unserem YAMLcheck etwas bezüglich defaults eingebaut. Geht es dabei um das was SeewolfPK gemeldet hat (siehe unten), oder ist das ein ganz anderes Thema? In der aktuellen 3.3 Doku finde ich jedenfalls nichts zu dem Punkt. Ein 4.0 Thema?

EXT läuft und zeigt in sehr sehr vielen EXT fehlenden Eintrag:

Code: Alles auswählen

    _defaults:
        public: true

Trotzdem funktionieren bei mir alle EXT.
Ist wohl erst bei zukünftigen Versionen wichtig?

Indicates that this extension may have a potential problem in a future version

edit: Referenzen die ich dazu direkt im Source finde, sind:

vendor\symfony\dependency-injection\Loader\Configurator\DefaultsConfigurator.php
vendor\symfony\dependency-injection\Loader\Configurator\Traits\ParentTrait.php
vendor\symfony\dependency-injection\Loader\YamlFileLoader.php

Wobei die letzte Datei mir am interessantesten scheint, bei diesem Thema.

[Mike-on-Tour]

Was SeewolfPK da gepostet hat, war nicht Grund der Änderung. Bin mir jetzt aber auch nicht sicher, was das bedeuten soll, da es mir m.W. noch nicht untergekommen ist.

[IMC]

Ich vermute mal das mit diesen Thema zu tun hat.
Marking Services as Public / Private

Mit fehlen aber leider zu viele Grundlagen um dies abschließend beurteilen zu können.

[LukeWCS]

Was SeewolfPK da gepostet hat, war nicht Grund der Änderung.

Dachte ich mir schon, wollte aber sicher gehen. Bei deinen Änderungen gehts ja primär um Prüfung auf Leerzeichen nach Doppelpunkt bei bestimmten Struktur-Konstellationen.

Bin mir jetzt aber auch nicht sicher, was das bedeuten soll, da es mir m.W. noch nicht untergekommen ist.

So geht's mir auch. Die letzte Version von David war 2.1.0-RC8, da war das definitiv noch kein Thema. phpBB33YAMLcheck prüft ja alles auch, was Davids Tool prüft, aber noch weitere Dinge und ist auch flexibler was YAML Struktur betrifft. Zumindest das Original hat ja ziemlich starres RegEx, wie die Version des neuen Coders da konzipiert wurde, weiss ich nicht. DLs gibts da offenbar nur nach Registrierung.

Das heisst das wurde vom neuen zuständigen Coder eingebaut. Und das scheint auch bei 3.3 "noch" gar kein Thema zu sein, sonst hätte man ja schon was darüber gelesen, in der Doku oder im Forum von .com. Dem Wortlaut der Meldung nach zu urteilen ("potential problem"), bezweifle ich eh, dass das eine Änderung ist, die noch bei 3.3 kommen wird. Sowas werden sie wohl nur bei einer neuen Minor Version von phpBB etablieren.

Handhabung der YAML Dateien ist ja nichts, was phpBB direkt betrifft, das ist Sache von Symfony. Das heisst für mich wiederum, dass es hier nicht um eine gewollte Änderung bei phpBB geht, sondern um eine notwendige Änderung bei Symfony.

edit: Sorry Thorsten, sah deinen Beitrag jetzt erst.

Ich vermute mal das mit diesen Thema zu tun hat.
Marking Services as Public / Private

Danke für den Link, interessant. Ich war wohl bei meiner Suche zu sehr auf "_defaults" fokusiert.

Mit fehlen aber leider zu viele Grundlagen um dies abschließend beurteilen zu können.

Geht mir auch so. Nach meinem Verständnis ist die Service Injection bei phpBB (genauer gesagt bei Symfony) einfach eine alternative Methode um Klassen einbinden zu können. Und in den Klassen unserer Exts legen wir ja ohnehin schon fest, was öffentlich und was privat ist. Mir erschliesst sich gerade nicht, warum man das nochmal extra in der YAML Datei festlegen kann/soll/muss.

[Mike-on-Tour]

Der Link von IMC bringt mich zu dem Schluss, dass das in neueren Ext nur unnötiger Ballast ist, denn wir binden unsere Klassen ja per Dependency Injection ein, und nicht über den phpBB-Container.
Zu Dependency Injection wird da ja auch explizit geschrieben, dass es nicht nötig ist.

Spätestens wenn Ext auch im ACP ausschließlich über Controller gesteuert werden müssen, hat sich das Thema erledigt.

[LukeWCS]

Der Link von IMC bringt mich zu dem Schluss, dass das in neueren Ext nur unnötiger Ballast ist, denn wir binden unsere Klassen ja per Dependency Injection ein, und nicht über den phpBB-Container.

Jain. Bei EMP muss ich zwingend eine bestimmte Klasse (Migrator) per Container einbinden, weil es ansonsten zu einem Fatal kommt, wenn man die Funktion " Berechtigungen des Benutzers testen" benutzt. Wurde mir von chris1278 gemeldet und von SeewolfPK bei ExtOnOff entdeckt. Ich habe das dann bei EMP gefixt und chris1278 hat meinen Fix dann bei ExtOnOff übernommen, da ich die betreffende Funktionalität schon bei ExtOnOff eingebaut hatte.

Und hier geht's doch um die Dependency Injection, bei der Einbindung mittels Container ist die YAML Datei doch gar nicht relevant?

Zu Dependency Injection wird da ja auch explizit geschrieben, dass es nicht nötig ist.

Spätestens wenn Ext auch im ACP ausschließlich über Controller gesteuert werden müssen, hat sich das Thema erledigt.

Dann frage ich mich, warum diese Prüfung überhaupt eingebaut wurde. Denn sie hat ja wohl weder jetzt noch zukünftig Relevanz.