[CI] Improvements

[Treggats]

Since we now support multiple databases, our CI should reflect that.

feature let github actions run with the following databases

• MySQL
• PostgreSQL
• SQLite

todo

• [x] merge GH Action workflows
• [ ] add services for the different database dialects
• [x] add job to run rector on pull request (created and synchronise type should suffice I think)
• [x] add rector to the composer qa script, before running the fixer

---

Proposal to support multiple database dialects

• move the database.php to an initalizer
• the initializer gets the intended dialect from the app config
• the app config has a property databaseDialect which reads its value from the env, and defaults to SQLite
• create a job in the workflow that only runs either on a pull request create event or a schedule
• add the env value to the Github Action workflow, and fill it based on the value of the matrix

[Treggats]

@aidan-casey is there a reason that 99% of the Quality Github Action workflows are the same?

By applying the following patch they could be merged into one.

patch

diff --git a/.github/workflows/quality-assurance.yml b/.github/workflows/quality-assurance.yml
index 34a0520..0cf2004 100644
--- a/.github/workflows/quality-assurance.yml
+++ b/.github/workflows/quality-assurance.yml
@@ -2,7 +2,11 @@ name: Quality Assurance
 
 on:
   pull_request:
-    branches: [main]
+    branches:
+      - main
+  push:
+    branches:
+      - main
   workflow_dispatch:
 
 jobs:
@@ -23,8 +27,9 @@ jobs:
       - name: Validate Composer
         run: composer validate
 
-  style:
+  check_style:
     name: Check Styling
+    if: ${{ github.event_name == 'pull_request' }}
     runs-on: ubuntu-latest
     steps:
       -   name: Checkout code
@@ -35,6 +40,24 @@ jobs:
           with:
             args: --config=.php-cs-fixer.dist.php --allow-risky=yes --dry-run -v
 
+  fix_style:
+    name: Check Styling
+    if: ${{ github.event_name == 'push' }}
+    runs-on: ubuntu-latest
+    steps:
+      -   name: Checkout code
+          uses: actions/checkout@v4
+
+      -   name: Run PHP-CS-Fixer
+          uses: docker://oskarstark/php-cs-fixer-ga
+          with:
+            args: --config=.php-cs-fixer.dist.php --allow-risky=yes -v
+
+      -   name: Commit Changes
+          uses: stefanzweifel/git-auto-commit-action@v5
+          with:
+            commit_message: Fixes styling.
+
   phpstan:
     name: Perform Static Analysis
     runs-on: ubuntu-latest
@@ -96,4 +119,4 @@ jobs:
         env:
           COVERALLS_REPO_TOKEN: ${{ secrets.GITHUB_TOKEN }}
         run: |
-          php-coveralls --coverage_clover=build/reports/clover.xml --json_path=build/reports/coveralls-upload.json -v
\ No newline at end of file
+          php-coveralls --coverage_clover=build/reports/clover.xml --json_path=build/reports/coveralls-upload.json -v

[Treggats]

@brendt how do you feel about adding rector as a job to keep it up to date?

[aidan-casey]

@Treggats they originally started as a single job, but as we considered things we might be looking to add in the future (automatic comments to PRs, etc.) it felt cleaner to split off.

[aidan-casey]

@Treggats does Rector have a CI mode like CS fixer? One of the decisions we made there was that those CI tools would not automatically write back to the repo on PRs, but would fail so people can run and commit them locally. We'd need to be able to maintain that approach, I think.

[Treggats]

@aidan-casey

@Treggats does Rector have a CI mode like CS fixer?

I'm guessing you mean running but not changing anything. It does. (--dry-run)

# the X at the end indicates an error exit code
#
~/P❯❯❯ vendor/bin/rector process --no-ansi --dry-run --no-diffs --no-progress-bar                    ✘ 1

 [OK] 1 file would have been changed (dry-run) by Rector 

About writing back you'd need a separate action for that. Like the fixer has.

but would fail so people can run and commit them locally.

I have the same approach on my work application, though we use Laravel Pint there, which will fail if it find anything

[Treggats]

@aidan-casey

@Treggats they originally started as a single job, but as we considered things we might be looking to add in the future (automatic comments to PRs, etc.) it felt cleaner to split off.

On the subject of splitting of, if we need to have recurring action/jobs/etc. it's possible to create our own custum action and reference that.

Small example

  - name: Deploy to staging
    uses: ./.github/actions/vapor-deploy
    with:
      composer_auth: ${{ secrets.COMPOSER_AUTH }

[aidan-casey]

On the subject of splitting of, if we need to have recurring action/jobs/etc. it's possible to create our own custum action and reference that.

Sure, but now you're still talking a minimum of two (if not three) separate actions. If we end up with three workflows that need the same functionality, it may make sense to refactor that, but I'm not seeing the need today.

[Treggats]

, it may make sense to refactor that, but I'm not seeing the need today.

Same, I just mentioned it as a possibility for in the future :)

[brendt]

I've made it so that MySQL tests work. We still need to update the github action, but I don't know how.

I'm gonna skip on adding pgsql support for now, see #354

[Treggats]

I'll update the GitHub action when I get back

[Treggats]

@brendt I've updated #322 to include MySQL in the CI (also Postgres, but could remove that again if needed) All tests passed

[Treggats]

With the PR being merged, this issue can be closed. The issue with Postgress is going to be addressed with #364