Provide (alternative) action for full phpcs run (on all files)

[indigoxela]

The phpcs action currently uses thenabeel/action-phpcs, which ... isn't actually maintained it seems.

In one of my own workflows I run phpcs on all files:

https://github.com/backdrop-contrib/i18n/blob/1.x-1.x/.github/workflows/code-checks.yml

With a custom reporter:

https://github.com/backdrop-contrib/i18n/blob/1.x-1.x/.github/misc/Github.php

(That reporter works better with GHA.)

But it also runs PHPStan, which might not be the right thing for every maintainer (and requires additional files, like phpstan.neon).

Here's one that combines phpcs and jshint (for projects with lots of custom JS):

https://github.com/backdrop-contrib/tinymce/blob/1.x-1.x/.github/workflows/code-checks.yml

Note that jshint also requires a .jshintrc file.

[laryn]

I wonder if we can provide all of these as options, with only phpcs defaulting to on, and the others behind an optional input flag.

We could follow the pattern you've used of just naming the action code-checks.yml. Then people could use it in different ways:

Simplest (just phpcs):

name: Coding Standards

on: [push, pull_request]

jobs:
  phpcs:
    uses: backdrop-ops/gha/.github/workflows/code-checks.yml@main

More complex (with example other options activated):

name: Coding Standards

on: [push, pull_request]

jobs:
  phpcs:
    uses: backdrop-ops/gha/.github/workflows/code-checks.yml@main
       with:
          run-phpstan: true
          run-jshint: true

[indigoxela]

simplest... more complex...

Problem: PHPStan most likely needs core and all dependencies in place (unless the module does zero API calls of any kind), so I wouldn't mix phpcs (needs no dependencies, code doesn't actually run) and PHPStan.
Not also that PHPStan might easily break, just because some core doc block got messed (happened recently).

Jshint needs it's own ruleset and that very likely differs per project - and probably isn't useful for most modules. We could postpone jshint until we get some more feedback on it. People might prefer ESLint, for example. And which directory should be checked, anyway (js/ or script/ or...).

Another culprit with phpcs: if a module ships with a PHP library, that lib should be excluded from checks - it's foreign code.

I'm also not sure, if we can mix the phpcs check via thenabeel/action-phpcs with a native check (without additional action), only by parameter.

Params we might need for phpcs: what to include/exclude and whether warnings should be errors.

My suggestion would be: start small.

• phpcs only on changed code (based on git blame - thenabeel/action-phpcs does that)
• phpcs on full code with params for ... exclude and the strictness...?

What do you think, @laryn? Does that sound feasible?

[laryn]

Starting small sounds like a good plan.