Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Autoload includes folder breaks composer invoked commands #1200

Open
datengraben opened this issue Mar 29, 2023 · 2 comments
Open

Autoload includes folder breaks composer invoked commands #1200

datengraben opened this issue Mar 29, 2023 · 2 comments
Labels
technical Non-functional changes (refactorings or increase test coverage)

Comments

@datengraben
Copy link
Contributor

Aktuell haben wir Konfigurationen für dev-tools wie PHPunit und PHP-CodeSniffer im Projekt eingecheckt. Aber wenn wir die Tools verwenden müssen wir die Binaries manuell runterladen und dann auch manuell aufrufen. Die Abhängigkeit zu den Tools können wir aber auch über composer mit require-dev und dann einen Aufruf des Tools unter scripts abbilden. Zum Beispiel wie yoast/wp-seo oder woocommerce/woocomerce das tun.

Damit verringert sich der Setup- und Dokumentation-Aufwand für das Plugin für Einrichtung und Verwendung der Standard-Tools, da alles bereits in einer interpretierbaren composer.json liegt und damit auch ausführbar ist.

Ein Problem steht aber noch im Weg: Die Composer.json enthält autoload Direktiven zu mehreren Dateien im includes Ordner. Diese Dateien scheinen mir Legacy-Code zu enthalten, welcher hauptsächlich in function-Blöcken definiert ist. Der dort definierte Code benötigt an ein oder anderer Stelle Methoden aus dem Wordpress-Core, welche aber zur autoload-Zeit noch nicht geladen sind, daher kann ich Befehle die über run-script definiert sind nicht ausführen und erhalten folgenden Fehler (welcher auf die noch nicht geladenen Wordpress-Core Sources hinweist):

PHP Fatal error:  Uncaught Error: Call to undefined function get_locale() in /home/chris/Code/commonsbooking/includes/OptionsArray.php:14
Stack trace:
#0 /home/chris/Code/commonsbooking/vendor/composer/autoload_real.php(41): require()
#1 /home/chris/Code/commonsbooking/vendor/composer/autoload_real.php(45): ComposerAutoloaderInite9b2cd7f9d792cf3f1e54768e93fc3ff::{closure}()
#2 /home/chris/Code/commonsbooking/vendor/autoload.php(25): ComposerAutoloaderInite9b2cd7f9d792cf3f1e54768e93fc3ff::getLoader()
#3 /home/chris/Code/commonsbooking/vendor/squizlabs/php_codesniffer/autoload.php(79): include('/home/chris/Cod...')
#4 [internal function]: PHP_CodeSniffer\Autoload::load()
#5 /home/chris/Code/commonsbooking/vendor/squizlabs/php_codesniffer/bin/phpcs(17): spl_autoload_call()
#6 {main}
  thrown in /home/chris/Code/commonsbooking/includes/OptionsArray.php on line 14

Da sehe ich zwei Probleme die zusammenhängen:

  • Setup unserer Tools ist nicht dokumentiertbar durch unsere composer.json (was best-practice zu seien scheint, s. woocommerce und yoast) einzig durch die Art und Weise wie Abhängigkeiten inkludiert sind
  • includes Ordner enthält dabei wichtigen Code für verschiedene Teile der Codebase. Der Code scheint aber nicht nach best-practices im Projek definiert zu sein (woocommerce und yoast scheinen beide pro Datei Klassen zu definieren aber keine public functions)

Mögliche Fixes:

  1. Enternen der includes aus dem autoload von composer und refactoren der includes in eigene Klassen. Dann können wir diese auch als ordentlich definierte Abhängigkeiten importieren. Hier ist für fraglich ob es besondere Gründe gab den Code in inlcudes zu platzieren.
  2. Irgendwie wordpress-core sources vor dem autoload laden. Ich habe aber noch keine Möglichkeit gefunden eigene imports vor dem composer-autoload zu bestimmen.

Hat jemand eine Idee oder kennt vielleicht eine bessere Lösung?

@datengraben datengraben added the technical Non-functional changes (refactorings or increase test coverage) label Mar 29, 2023
@hansmorb
Copy link
Contributor

Also die OptionsArray hätte ich sowieso gerne aus den Includes raus, spätestens ab Merge von #1161 wäre das auch mit drin. Bei den anderen bin ich mir nicht sicher.

@chriwen
Copy link
Member

chriwen commented Apr 1, 2023

Diese Codesniffer-Sachen sind glaube ich auf meinem Mist gewachsen und können da gerne wieder herausgenommen werden. Ich bin da nicht so ganz firm in der Sache, wenn du @datengraben da ne bessere Lösung hast, gerne.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
technical Non-functional changes (refactorings or increase test coverage)
Projects
Development

No branches or pull requests

3 participants