Conversation
- Fix 107 implicitly nullable parameters across codebase
- Add #[AllowDynamicProperties] to ApplicationCore, Application, Config
- Replace deprecated each() with foreach (5 instances)
- Modernize UTF-8 encoding functions (45+ instances)
- Fix curly brace string access syntax (9 instances)
- Remove deprecated ${} string interpolation syntax
- Remove deprecated curl_close() calls
- Fix undefined variable bug in shopimporter_shopware6.php
Dependencies:
- Update Guzzle 6 -> 7.10
- Update Smarty 3 -> 4.5
- Update aws-sdk-php to 3.332 (security)
- Update phpmailer to 6.9 (security)
- Update tcpdf to 6.7 (security)
Tested:
- Syntax check: 2440 files, 0 errors
- Composer dependencies resolved
|
Vielen Dank! Das ist eine Menge :-) Ich habe vor einigen Monaten die composer.json überhaupt in einen brauchbaren Zustand gebracht und zunächst von Paket-Updates abgesehen, da in den Changelogs eine Breaking Changes auftauchten. Ich war mir nicht sicher ob uns die betreffen und hatte da keine Zeit das zu prüfen. Hast du das im Rahmen deiner Änderungen gemacht oder sollten wir das noch tun? |
|
Also ich habe es soweit geprüft wie ich konte, sprich es einmal mit meinen Daten durchlaufen lassen und geschaut ob irgendwo fehler auftauchen, aber ich kann bei weitem nicht jeden Edgecase abdecken, entsprechend schau es dir bitte auch nochmal an. |
|
Alles klar, mache ich. Jeden Edgecase werde ich auch nicht abdecken können, aber ich schaue dann nochmal gezielt an die Stellen die mir damals Sorgen gemacht hatten... |
| "fiskaly/fiskaly-sdk-php": "1.2.100", | ||
| "guzzlehttp/guzzle": "^6.5.5", | ||
| "laminas/laminas-mail": "^2.12.5", | ||
| "guzzlehttp/guzzle": "^7.8", |
There was a problem hiding this comment.
Laut guzzle gibt es Support für PHP 8.5 erst ab Version 7.10:
https://github.com/guzzle/guzzle/releases
Spricht etwas gegen 7.10?
| "ext-soap": "*", | ||
| "aura/sqlquery": "2.7.1", | ||
| "aws/aws-sdk-php": "3.175.2", | ||
| "aws/aws-sdk-php": "^3.332.0", |
There was a problem hiding this comment.
Warum hast du hier die Version 3.332 ausgewählt und nicht die neueste (3.368)?
| "swiss-payment-slip/swiss-payment-slip": "0.13.0 as 0.11.1", | ||
| "swiss-payment-slip/swiss-payment-slip-fpdf": "0.6.0", | ||
| "tecnickcom/tcpdf": "6.3.5", | ||
| "tecnickcom/tcpdf": "^6.7", |
|
ich werde mal schauen ob deine versionen auch laufen oder ob es da probleme gibt, ansonsten update ich das repo entsprechend |
|
Das wäre prima! Vielleicht gibt es auch einen guten Grund, ich konnte nur keinen offensichtlichen erkennen. |
|
Hier die KI zusammenfassung nach dem Test der Versionen Änderungen |
|
"laminas/laminas-mail": "^2.12.5", - kann eigentlich raus wegen php mailer |
|
Danke dir! Ja, diese Mail Geschichten müsste man mal vereinheitlichen... aber das ist ne andere Baustelle :-) |
ich arbeite da auch noch an etwas richtig großem, wo ich hoffe dass es nöchste woche fertig wird, da bin ich mal aufs feedback gespannt :D |
|
Achso, pushst du noch den commit mit den neuen Versionen? |
# Conflicts: # composer.json
habs in die dokumentierte branch gepushed xD |
|
Leider hat der PR merge conflicts, kannst Du das bitte ansehen und ggf. beheben? |
uff ich sehe es gerade, ich schau es mir an |
|
ich habe laminas zum großteil durch symfony/mailer + symfony/mime ersetzt, nur ein fork für die imap schnittstelle ist noch da, ich habe das jetzt noch nicht geprüft, aber werft gerne mal einen blick drauf |
|
Okay, das ist auf der einen Seite ein wirklich guter Schritt, auf der anderen Seite erfordern wir damit jetzt effektiv php 8.2. Hier sollten wir jetzt einmal abwägen... wenn wir PHP 8.5 unterstützen können ohne PHP 8.1 zu verlieren würde ich das erstmal vorziehen und dann die Erhöhung der Mindestversion auf bspw. 8.3 in einem separaten Schritt mit Ankündigung vornehmen. Kriegen wir diese beiden Dinge mit vertretbarem Aufwand getrennt? |
|
PHP 8.1 ist doch seit Januar unsupported, sehe ich eher kein Problem damit. Evtl. sollten wir noch Grips in einen Major-Version-Wechsel stecken, sodass niemand durch ein Upgrade sein System zerschießt und dann in einer Notfalloperation PHP upgraden muss. |
|
Ja, genau darum ging es mir. Ich will nur vermeiden, dass jemand ein Update zieht und danach ein Problem hat :-) |
|
Ich hätte gerne eine Funktion mit der man gezielt Systemupgrades machen kann, inkl. Beschreibung, Systemanforderungscheck, Datenmigration etc. Evtl. ist es ein guter Zeitpunkt das jetzt anzugehen? |
|
Was genau meinst du mit "Systemupgrades"? In meinem Workflow würde das meiste von docker abgedeckt, weil ich einen entsprechenden Container benutze der bereits das richtige php mit apache, extensions und dem entsprechenden OpenXE-Code mitbringt. Bei mir wäre da nur die Frage der Datenmigration, wo es ja einige Best-practices gibt und man sich für einen Weg entscheiden sollte |
|
Ich meine OpenXE-Upgrades. Also das was momentan mit git und danach dem Datenbankupgrader gemacht wird. Das rammelt ja einfach die neueste Version drauf und zieht die DB auf links. Da wäre es schön wenn man in minor und major upgrades unterscheiden kann und bei major wird ein Check gemacht, ob das System überhaupt kompatibel ist. Und was momentan komplett fehlt ist das Transformieren von Daten, z.B. weil Tabellen anders strukturiert sein sollen. |
|
Okay, alles klar. Ja, strukturierte DB-Upgrades würde ich mir auch sehr wünschen und ich könnte mir vorstellen, dass die laravel-migrations hier eine gute Hilfe für uns sein können, aber da hat @Avatarsia vielleicht noch eine bessere Idee?! Eine Unterscheidung zwischen minor und major Updates können wir mit dem aktuellen Git-Ansatz ja eigentlich recht einfach umsetzen, wenn wir bspw. in Git mit tags arbeiten. Dies hätte auch den Vorteil, dass wir den master branch nach einem großen Merge (wie diesem hier) nochmal in Ruhe testen können, bevor wir einen Tag erstellen und die Version damit "freigeben" Gleichzeitig können wir ausgehend von den Tags dann docker-images erzeugen, die immer die nötigen Systemvorraussetzungen mitbringen und insbesondere für "Neueinsteiger" einfach zu nutzen sind. Gleichzeitig können aber alle Nicht-Docker-Nutzer mit ihrem bisherigen Ansatz weiterarbeiten. Das Git-Update-Theme müsste auch relativ einfach umzusetzen sein, oder? |
|
eigentlich wollte ich das erst prasentieren wenn ich damit weiter bin, werft mal einen blick auf meinen rework branch |
|
https://github.com/Avatarsia/OpenXE/tree/rework localhost:3001/de/login openxe@local.de im modern ordner docker-compose up --build -d es sind aber bis jetzt gerade mal 50% aller module implementiert |
|
Oha! Puh... Ich bin jetzt nicht sicher, was ich dazu sagen soll... ich versuche es aber mal: Auch ich habe bereits Arbeit in eine Modernisierung des JS-Codes gesteckt, dabei aber auf Vue gesetzt weil ich das lieber mag als React. Ist vermutlich zu 90% eine Geschmacksfrage, aber ist natürlich ungünstig, dass hier zwei Leute in unterschiedliche Richtungen arbeiten. PostgreSQL als Datenbank finde ich eine gute Idee. Aber die wohl kontroverseste (gibt es das Wort?!) Entscheidung wird wohl die Implementierung des Backends ebenfalls in TypeScript sein. Ich vermute, dass du damit einen großen Teil der aktuellen Mitstreiter verlieren wirst. Während es natürlich für viele Entscheidungen gute Argumente gibt, bin ich kein Freund von JS/TS im Backend. Vielleicht magst du ja mal deine Überlegungen teilen, warum du dich für diesen Weg entschieden hast und nicht bspw. einen Wechsel hin zu laravel oder symfony, was sicherlich ein deutlich kleinerer "Bruch" gewesen wäre. |
ich hab n bissl ping pong mit ein paar llm's gespielt um zu schauen was die modernsten ansätze sind die in zukunft auch weiterentwickelt werden und das beste portfolio aus leistung und sicherheit bringen und bin im endeffekt bei diesem setup gelandet. Backend (Core) ich habe auch die laravel-migrations diskusion im forum gesehen, da gab es aber einige punkte die dagegen sprachen (welche habe ich nicht mehr im Kopf). Da das ganze Backend als API-Schnittstelle fungiert, würde es wahrscheinlich auch kein großartiges Problem sein, dort Vue zu nutzen. Für mich war das einfach nur ein Versuch, zu gucken, wie weit ich damit komme und ob ich sowas umgesetzt bekomme. Hat keinerlei Relevanz, ob das am Ende wirklich durchgreift, weil viele der Sachen auch noch nicht wirklich so im Frontend ineinandergreifen, wie ich das gerne hätte. Und der andere Punkt, der mich eigentlich ein bisschen gestört hat die ganze Zeit, ist, immer wenn ich etwas versucht habe zu integrieren, dann hat es meistens irgendwo das UI zerschossen und das hat mir überhaupt keinen Spaß gemacht. |
|
Lieben Dank für die Erläuterung. Ich bin in vielen Punkten ganz bei dir. Vue vs React ist wie gesagt in vielen Fällen eine Geschmacksfrage. Wenn man das lange genug analysiert, findet man bestimmt die "bessere" Alternative für dieses Projekt, aber ich glaube man erreicht in beiden Fällen ein gutes Resultat. Beim Backend bzw. der API würde ich sagen NestJS, Nuxt oder auch verschiedene Libs in Golang oder gar ASP.NET sind deutlich "moderner" als PHP, bieten async und sind event-driven. Wenn man also auf der völlig grünen Wiese beginnt würde ich das auch in Erwägung ziehen - auch wenn JS/TS nicht mein Liebling ist und mir das im Frontend völlig ausreicht ;-) Aber auch das ist natürlich ein Stück Geschmackssache. Neben der Technologie-Perspektive der LLMs würde ich aber hier ganz klar auch die menschliche Komponente berücksichtigen. Alex ist beispielsweise viel konventioneller als ich es bin. Wenn es nach ihm geht, dann so wenig JS wie möglich :-) Deine Enttäuschung über das Zerschießen der UI kann ich sehr gut nachvollziehen und das würde ich gerne auch verbessern. Ich weiß noch nicht wohin die Reise führen wird, was ich mir aber gut vorstellen könnte wäre eine Kombination aus dem was da ist, ein neues Fundament aus Symfony (oder Laravel) und eine modularere UI mittels Tailwind CSS. Meiner aktuellen Einschätzung nach wäre das ein guter Kompromiss um die PHP-Entwickler abzuholen, alles mehr zu strukturieren und zu modernisieren und eine moderne UI zu haben. So haben dann vielleicht möglichst viele Spaß an der Mitarbeit. Aber die Diskussion hat jetzt doch immer weniger mit dem eigentlichen PR zu tun. Da das rework-Projekt sicherlich nicht kurzfristig "fertig" sein wird, sollten wir hier das Update auf PHP 8.5 zu Ende bringen und die Diskussion über die zukünftige Entwicklung von OpenXE vielleicht ins Forum verlagern. Da werden sich sicherlich noch weitere Entwickler einbringen und wir bekommen auch ein besseres Bild. Meine Aussagen zuvor sind ja auch überwiegend Vermutungen. |
|
Ich habe in #231 einen einfachen Pre-Upgrade-Check implementiert der als Voraussetzung für diesen PR ausreichen sollte |
3 participants
Dependencies:
Tested:
mit dokumentation in diesem repo https://github.com/Avatarsia/OpenXE/tree/php85-upgrade-with-docs