chore: phpcs using config in github actions, restore: phpcs.xml.dist#26
chore: phpcs using config in github actions, restore: phpcs.xml.dist#26Tomcraft1980 merged 4 commits intomodified-shop:mainfrom
Conversation
.github/workflows/php-code-style.yml
Outdated
| # includes/*.php = Prüft alle PHP-Dateien im includes-Ordner | ||
| - name: Check PHP Code Style (PSR-12) | ||
| run: phpcs --standard=PSR12 *.php admin/*.php includes/*.php | ||
| run: php-cs-fixer fix --config=.php-cs-fixer.dist.php --show-progress=none --diff --dry-run |
There was a problem hiding this comment.
Ich denke, dass das so noch keine gute Idee ist. Wenn ich das zum Test ausführe erhalte ich folgende Meldung.
PHP CS FIXER
Running analysis on 1 core sequentially.
You can enable parallel runner and speed up the analysis! Please see usage docs for more information.
Loaded config default from ".php-cs-fixer.dist.php".
Using cache file ".php-cs-fixer.cache".
1) admin/send_email.php
---------- begin diff ----------
--- /Users/robin/test/admin/send_email.php
+++ /Users/robin/test/admin/send_email.php
@@ -8,7 +8,7 @@
// MailSender functions
require_once __DIR__ . '/../includes/send_email_functions.php';
-use App\Service\MailSender;
+use App\Service\MailSender;
// E-Mail-Konfiguration
$to = 'info@modified-shop.org';
----------- end diff -----------
Found 1 of 11 files that can be fixed in 0.010 seconds, 16.00 MB memory used
PHPCS
FILE: /Users/robin/test/admin/send_email.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
11 | ERROR | [x] Whitespace found at end of line
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
- Wird bei
php cs fixernicht gesagt, gegen welche Regel hier verstoßen wird und ... - Fehler die nicht automatisch gefixt werden können, werden nicht angezeigt.
Meine aktuelle Meinung ist: php cs fixer ist eine gute Idee und funktioniert auch aber für eine reine Prüfung im Pull Request bietet phpcs den klareren Vorteil dass es exakt benennt welche Regel verletzt wurde und in welcher Zeile. php cs fixer zeigt im dry run zwar die Änderungen die nötig wären doch es liefert nur den Patch ohne die konkrete Regel dahinter. Hier ist z.B. nicht sichtbar, dass das Problem das Leerzeichen am Ende war. Für Reviewer ist das weniger eindeutig weil man zwar sieht dass etwas geändert werden müsste jedoch nicht warum. phpcs liefert dagegen eine saubere Meldung mit Sniff Name Dateiangabe und Zeile wodurch Verstöße sofort nachvollziehbar werden und sich schneller korrigieren lassen. Genau diese klare Zuordnung macht phpcs als reines Prüfwerkzeug in der CI aus meiner Sicht besser.
Ich muss aber auch sagen, dass ich nicht weiß, ob sich das mit php cs fixer nachbilden lässt.
There was a problem hiding this comment.
Bekommen wir einen lauffähigen Zustand wieder her? 😉
Ich bin da gerade ein wenig überfordert und auf eure Expertise angewiesen.
There was a problem hiding this comment.
@Tomcraft1980 Werden wir bekommen. Ich würde gerne noch abwarten was @dgodglueck sagt. Aktuell finde ich "meine" Variante noch besser, aber ich habe die Weisheit nicht gepachtet. Vielleicht hat er noch einen guten Einwand.
There was a problem hiding this comment.
Ich danke euch beiden sehr! 👍
There was a problem hiding this comment.
Mit dem jetzigen PR wird alles wieder passen 👍
Ich finde tatsächlich die Lösung mit dem phpcs Tool auch übersichtlicher, und macht am Ende das gleiche, wieder was neues gesehen und gelernt
There was a problem hiding this comment.
phpcs --standard=PSR12 *.php admin/*.php includes/*.php
Hatten wir mal zur Demo, aber es bezieht noch nicht die phpcs.xml.dist im Root Verzeichnis mit ein. Da die phpcs.xml.dist erst später hinzugekommen ist als "Beispiel".
Man könnte es also so anpassen, dass man phpcs statt der Option --standard=PSR12 die Datei phpcs.xml.dist mitgibt. Ich glaube man kann --standard=PSR12 als Option einfach weglasse und dann nimmt er automatisch die phpcs.xml.dist aus dem Root. Aber für die CLI wäre es vielleicht leichter oder besser verständlich / leserlicher, wenn man direkt sagt, er soll die phpcs.xml.dist nehmen.
There was a problem hiding this comment.
Hinweis: Es geht phpcs.xml.dist als auch .phpcs.xml.dist (mit Punkt) wir haben glaube ich OHNE Punkt.
There was a problem hiding this comment.
@RobinTheHood soweit ich verstanden habe, werden die Tools im Rootverzeichnis ausgeführt, wo auch die phpcs.xml.dist liegt, aber ich hatte mit dem cs-fixer identisch Problem. Also wenn ich die Konfiguration nicht explizit angebe, dass das Tool (beliebiges) sagt es benötigt eine.
There was a problem hiding this comment.
@dgodglueck Ich glaube du schreibst aktuell den Dateinahmen mit Punkt --standard=.phpcs.xml.dist müsste wohl aber --standard=phpcs.xml.dist ohne Punkt sein.
This reverts commit 2b8a3c0.
d0fceab to
3c582cb
Compare
|
Danke euch beiden! |
No description provided.