Add GitHub Action for PHP Code Style checks #18
Conversation
|
Halleluja, hier wird es komplex... |
@Tomcraft1980 Wir können das ganz klein auseinander nehmen und erklären und wir können es auch komplett weg lassen. Ist nicht wichtig und für den Anfang auch vielleicht zu viel. Aber dann hat man es einmal gesehen. In der täglichen Arbeit kann es viel Arbeit abnehmen. Aber es soll auch nicht überfordern. |
|
Ich erkläre einmal was hier passiert ... |
|
Wichtig wäre erstmal zu wissen, was zu tun ist um die Checks erfolgreich durchlaufen zu lassen. 😉 |
|
@Tomcraft1980 Ich habe dort ein Skript abgelegt, das ich php-code-style.yml genannt habe. In diesem Skript habe ich u. a. Folgendes definiert: # Name der GitHub Action, der im Actions-Tab angezeigt wird
name: PHP Code StyleDas ist einfach ein Name, den ich mir ausgedacht habe. Er wird beim Klick oben im Reiter Actions und im PR unter den Checks angezeigt. Dann habe ich festgelegt, dass die Action nur gestartet werden soll, wenn ein Pull Request erstellt oder aktualisiert wird, der in den main-Branch gemerged werden soll: # Trigger: Wird bei jedem Push zu einem Pull Request ausgeführt
on:
pull_request:
branches: [ main ] # Nur PRs zum main-BranchIn der Action wird dann definiert, auf welchem Betriebssystem der Job laufen soll. GitHub startet dafür einen frischen Runner. Ich möchte, dass alles auf der neuesten Ubuntu-Linux-Umgebung läuft: jobs:
phpcs:
runs-on: ubuntu-latest # Ubuntu Linux ContainerJetzt kann ich Schritt für Schritt festlegen, welche Befehle bzw. Unterskripte auf diesem Ubuntu-Runner ausgeführt werden sollen. Als Erstes möchte ich den Code des PR „auschecken“. Theoretisch könnte ich das mit git clone usw. selbst machen, aber dann müsste ich mühsam die Repo-URL und die PR-Referenz herausfinden. GitHub bietet dafür eine fertige Action an: actions/checkout@v4. Die rufe ich einfach auf, und sie erledigt das für mich: steps:
# Schritt 1: Code aus dem Repository laden
# Hier verwenden wir eine vorgefertigte Action von GitHub
- uses: actions/checkout@v4Bis hierhin hat GitHub nur einen leeren Ubuntu-Runner bereitgestellt. PHP und Tools wie phpcs sind dort aber nicht zwingend so installiert, wie ich sie brauche. Dafür gibt es in der PHP-Welt eine sehr praktische Action von shivammathur, die shivammathur/setup-php@v2 heißt. Damit kann ich einfach angeben, welche PHP-Version und welche Tools installiert werden sollen. Jedem Schritt kann ich btw. optional einen Namen geben, hier z. B. „Setup PHP“, dem Step zuvor haben ich keinen Namen gegeben. # Schritt 2: PHP installieren und phpcs einrichten
# Hier verwenden wir eine vorgefertigte Action von shivammathur
- name: Setup PHP
uses: shivammathur/setup-php@v2
with:
php-version: '8.2' # PHP Version
tools: phpcs # PHP CodeSniffer installierenJetzt ist der PR-Code ausgecheckt und PHP 8.2 plus phpcs sind installiert. Im nächsten Schritt kann ich phpcs aufrufen. Hier lasse ich phpcs alle PHP-Dateien nach dem Standard PSR-12 prüfen und gebe vorsichtshalber explizit die zu prüfenden Ordner an. Diesen Schritt nenne ich z. B. „Check PHP Code Style (PSR-12)“: - name: Check PHP Code Style (PSR-12)
run: phpcs --standard=PSR12 *.php admin/*.php includes/*.phpWenn jetzt eines der verwendeten Tools/Programme/Befehle
mit einem Fehler (also einem Exit-Code ungleich 0) endet, wird der Job als fehlgeschlagen markiert. Im PR sieht man dann so etwas wie: 
Und in der Übersicht steht dann in der Form:
Also in unserem Fall z. B.:
Das bedeutet: In der Action „PHP Code Style“ (den Namen haben wir ja frei gewählt) ist der Schritt mit dem Programmaufruf Was muss man also jetzt machen?
Hinweis: Da wir anfangs noch nicht konsequent auf PSR-12 geachtet haben und die Action der Einfachheit halber alle Dateien prüft (nicht nur neue oder geänderte), tauchen jetzt natürlich sehr viele Meldungen auf. Das ließe sich später noch gezielt verbessern. |
|
Hier mal ein Beispiel aus der Ausgabe: Er sagt hier:
Aktuell (falsch) <?php
echo 1; // Debug
echo 1;
...Richtig wäre: <?php
echo 1; // Debug
echo 1;
... |
|
Danke dir für die super verständliche Erklärung! 👍
Da wird den Code gerade für PHP 8.4 fit machen, wird es Sinn machen hier 8.4 zu verwenden. Sowas wie: - name: Check PHP Code Style (PSR-12)
run: phpcs --standard=PSR12 *.php */*.phpUnd was ist mit Unterverzeichnissen? Werden die automatisch mit berücksichtigt?
Ich meine, dass Gerhard hier den Code nach dem Import ins Git einmal komplett über VS Code formatieren wollte. Da gibt es wohl ein Plugin für, welches das erledigt. Das wird uns die Arbeit später sicherlich erleichtern. P.S.: Wie und wo muss ich jetzt die Dateien ändern, damit sie durch die Checks laufen? |
Tomcraft1980
left a comment
Tomcraft1980
left a comment
There was a problem hiding this comment.
Siehe inline Kommentar.
|
Ich habe jetzt testhalber mal auf PHP 8.4 gestellt.
Das klingt für mich ehrlich gesagt am besten. Also alles rekursiv und dann Ausnahmen für die Dinge, die wir von extern eingebunden haben, sowas wie Smarty, GuzzleHttp, Psr oder Magnalister
Ich habe jetzt mal testweise hier direkt in den PR die Änderung bzgl. PHP 8.4 rein gebracht. Scheint funktioniert zu haben. |
| uses: shivammathur/setup-php@v2 | ||
| with: | ||
| php-version: '8.2' # PHP Version | ||
| php-version: '8.4' # PHP Version |
There was a problem hiding this comment.
Sehr gut. Der Check schlägt zwar fehl, weil die PHP Dateien noch nicht alle PRS12 sind. Aber das Tool hat kein PHP Error, weil es nicht unter PHP8.4 läuft. 8.4 scheint zu funktionieren. 🙂
There was a problem hiding this comment.
Jepp, können wir wohl erstmal so lassen. 🎉
|
@Tomcraft1980 Wäre jetzt spannend zu sehen was passiert, wenn der PR #19 gemerged wird, der einige Coding Style Fehler die hier der Check bemängelt beheben sollte. Dann sollte hier der Check durchlaufen und grün angezeigt werden. Btw.: da die Action bis jetzt nur hier im PR enthalten ist, und ihr den PR noch nicht freigeben habt / in |
|
Da gibt es einen Error |
|
Ach ja, ich hatte ja noch zur Demo ein WARNING übrig gelassen. Ich hatte vergessen in der Action zu sagen, dass WARNINGs nicht zu Fehlern führt. Sorry mein Fehler. Wenn man sich den Check im Detail ansieht, sieht man, dass er nur noch ein einziges WARNING bemängelt. |
|
Was war denn das Warning? P.S.: Unter dem Reiter "Actions" sind nun 4 Actions und nur die letzte lief durch. |
|
@Tomcraft1980 Man kann in der Config Datei |
|
@Tomcraft1980 Ich habe mir da noch nie Gedanken drüber gemacht, ob es sinnvoll ist die ehemaligen Action-Runs zu löschen. 😅 |
This reverts commit 90aeb9f.
Ich habe die Warnings mal wieder mit rein genommen, will mir das mal anschauen.
Ah okay das ist also nur das log bzw. die History der vergangenen Durchläufe. Ich dachte dort ist die Action, aber die liegt ja in /.github/workflows/php-code-style.yml |
|
Ich habe gerade einen PR versucht. Zugriff verweigert. Jetzt weiß ich nicht, ob das am Projekt liegt oder ich generell ein Problem habe mit GitHub. |
|
Uffff... das war ne schwere Geburt.
Wir müssen ja nicht gleich alle Warnings abschalten, sondern erstmal nur die SideEffects und dann mal schauen, was da noch so kommt. 😉
Wo findet man denn diese Datei? |
Sicher, dass du einen PR versucht hast und nicht einen direkten Push in den main Branch? |
|
@Tomcraft1980 @1bigQ Ich beantworte gleich beide eure Fragen, bin nicht ganz so schnell beim Schreiben. Schon mal kurz: @1bigQ hat keine Schreib recht um einen Branch den er bei sich lokal macht direkt in das Repo zu Pushen, weil der nicht Mitglied der Organisation ist. Er müsste also ein Fork machen, den Branch dann in seinem Fork pushen und dann dann daraus in PR machen. Gleich mehr ... |
|
Check. Dann erstelle ich erstmal einen Fork. Dachte das ginge ohne. |
|
Ok. Der PR aus dem Fork ist anscheinend angekommen. Muss das dann immer über GitHub laufen? Sonst schaue ich mal wo ich das im Programm einstellen kann, dass es über das Programm gehen könnte. |
|
@Tomcraft1980 Die Datei |
|
Wieso ist das fehleranfälliger? Der Pfad wird doch nur einmalig in der /.github/workflows/php-code-style.yml gesetzt oder nicht? Da erfolgt doch der Aufruf von phpcs dann automatisch. |
@Tomcraft1980 @modGTB PHP CS Fixer ist ein reines „Aufräum-Werkzeug“ oder auch Laravael Pint. Es passt nur den Schreibstil des Codes an, also Dinge wie Einrückungen, Leerzeilen, Klammern oder die Reihenfolge der use-Statements. Es sorgt dafür, dass alles nach einem einheitlichen Standard wie PSR-12 aussieht. Es verändert dabei nie die eigentliche Logik. Der Code sieht danach einfach ordentlicher aus, funktioniert aber genau wie vorher. Dieses Tool eignet sich ideal für Team-Projekte, weil jeder Entwickler automatisch denselben Stil einhält ohne das drauf achten muss, da PHP CS Fixer es immer automatisch korrigiert (wenn möglich). Rector dagegen geht viel viel weiter. Es ist kein Style-Tool, sondern ein Werkzeug für Refactorings. Damit kann man den Code automatisch modernisieren oder auf eine neue PHP-Version heben, ohne alles per Hand umzuschreiben. Rector kann zum Beispiel alte Funktionen durch moderne Sprachfeatures ersetzen, Arrays auf die neue []-Syntax umstellen, veralteten Code reparieren toten Code erkennen und löschen oder sogar beim Umstieg von PHP 7 auf PHP 8 helfen. Es nimmt also strukturelle und teilweise auch logische Änderungen vor, nicht nur Schönheitskorrekturn. Wenn man nur möchte, dass der Code "hübsch und einheitlich" aussieht, nimmt man PHP CS Fixer oder Laravel Pint. Wenn man den Code automatisch modernisieren, aktualisieren oder strukturell verbessern möchte, nimmt man z. B. Rector. In vielen Projekten verwendet man sogar beide Programme: PHP CS Fixer für den Stil und Rector für langfristige Verbesserungen und Modernisierungen. |
|
@Tomcraft1980 Welchen Coding Stand möchtet ihr denn gerne? PSR12 (gängige Praxis) als Basis und erst einmal etwas aufweichen, damit einem nicht alles um die Ohren fliegt? |
@Tomcraft1980 Das kommt etwas auf die Sichtweise an. Betrachtet mach nur die Aktion / Workflow. Dann ist es im Grunde egal. Wenn du aber z. B. mit einem Editor wie VS Code oder PHPStorm arbeitest, dann suchen diese die Datei für gewöhnlich im Root Verzeichnis. VS Code zeigt mir die Fehler und Warnings bereits beim Schreiben mit Hilfe von phpcs an. Liegt die Datei nicht im Root. Muss ich den Editor erst nachkonfigurieren. Kein Beinbruch und machbar. Aber es ist auch schön wenn es out-of-the-box funktioniert. 
|
Danke dir! Ich denke die Tools wird sich Gerhard dann mal in VS Code anschauen.
Ja, genau so. 👍
Alles klar, dann vielleicht doch lieber im Root, wo die IDEs die Datei automatisch finden und verwenden. |
@Tomcraft1980 siehe PR #23 zum Thema |
|
Danke dir! |
Introduce a GitHub Action that runs PHP CodeSniffer to enforce PSR-12 coding standards on PHP files during pull requests.
See: #14 (comment)