Skip to content

Lint PHP and report Composer package changes in CI - #26

Open
tomjn wants to merge 8 commits into
masterfrom
basic-gh-action-linting
Open

tomjn wants to merge 8 commits into
masterfrom
basic-gh-action-linting

Conversation

@tomjn

@tomjn tomjn commented Aug 6, 2025 •

Copy link
Copy Markdown
Contributor

Adds a basic linting action for PHPCS

This PR also turns hm-linter off for this repository, because per-repository Actions are now preferred. The workflow covers everything hm-linter checked here, except annotations on fork PRs and linting branches that have no PR.1

Footnotes

  1. What hm-linter covered, and what replaces it:

    • PHPCS with the HM standards and phpcs.ruleset.xml: the Linting workflow runs it with the same ruleset.
    • ESLint and stylelint: the repo has no .js or .css files, so nothing is lost. The CSS in inc/admin-bar.php is a PHP string, which hm-linter never checked. If the plugin gains front-end assets, add both to the workflow.
    • Annotated check run on PRs: lint-action posts one for PRs from this repository. Fork PRs run composer lint instead, so the check still passes or fails, but errors appear only in the job log.
    • Pushes to any branch: the workflow runs on PRs and pushes to master only. A branch with no PR gets no lint.
    • Standards version: hm-linter used the latest HM standards. The workflow uses the version in composer.lock, so CI matches a local composer lint, and new rules arrive through dependency updates.
    • First-install issue listing existing errors: not needed, because master lints clean.

    .github/hmlinter.yml disables all three hm-linter checks. The app stays installed at org level. ↩

@tomjn tomjn mentioned this pull request Aug 6, 2025
The linting workflow only had a workflow_call trigger and nothing
called it, so it never ran. Run it on pull requests and on pushes to
master. Pin setup-php to a SHA and move every action to its latest
release.

Co-Authored-By: Claude <noreply@anthropic.com>
@svandragt svandragt changed the title Add basic linter action Lint PHP and report Composer package changes in CI Oct 16, 2026
svandragt and others added 2 commits October 16, 2026 10:52
Co-Authored-By: Claude <noreply@anthropic.com>
humanmade/coding-standards 2.x, which the WPCS advisory fix needs,
requires PHP 8.2 to run. The ruleset still checks compatibility with
PHP 7.2.

Co-Authored-By: Claude <noreply@anthropic.com>

@svandragt svandragt left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lint now runs and passes. Two problems needed fixing before this could merge:

  • PRs from forks get a read-only token. lint-action can't create its check run, and the sticky comment can't post, so both jobs fail on fork PRs whatever the code looks like.
  • If a later push reverts composer.lock, the old package-change comment stays on the PR.

I've pushed fixes for both. Fork PRs now run composer lint without the check run annotations.

This workflow also replaces hmlinter: we now prefer a per-repo GitHub Action over hm-linter. This PR adds .github/hmlinter.yml to turn hm-linter off for this repo, so each PR gets one PHPCS report.

svandragt and others added 2 commits October 16, 2026 11:50
Fork PRs get a read-only GITHUB_TOKEN, so lint-action can't create its
check run and the sticky comment can't post. Run plain composer lint on
forks instead, and skip the comment there. Delete the package-change
comment when a later push leaves composer.lock with no changes.

Co-Authored-By: Claude <noreply@anthropic.com>
The Linting workflow now runs phpcs on every PR, and per-repository
Actions are preferred over hm-linter. Disable all three hm-linter
checks so contributors get one lint report.

Co-Authored-By: Claude <noreply@anthropic.com>
@svandragt
svandragt requested a review from roborourke October 16, 2026 10:56
@svandragt

Copy link
Copy Markdown
Contributor

@tomjn I've picked this up. Changes since your last commit:

  • The Linting workflow now runs on pull requests and pushes to master. It only had workflow_call before, so it never ran.
  • phpcs is on PATH and uses phpcs.ruleset.xml. lint-action calls a bare phpcs, so the first run failed with "PHP_CodeSniffer is not installed".
  • Lint runs on PHP 8.2. The coding standards update in Fix the WPCS code execution advisory and require PHP 8.2 #31 (Fix the WPCS code execution advisory and require PHP 8.2) needs it.
  • Every action is SHA-pinned at its latest release, including setup-php.
  • Fork PRs run composer lint and skip the PR comment, because their token is read-only.
  • The Composer diff comment is deleted when a later push leaves no package changes.
  • .github/hmlinter.yml turns hm-linter off, so each PR gets one PHPCS report.

CI is green. This should merge before #31.

@svandragt
svandragt marked this pull request as ready for review October 16, 2026 10:58
@svandragt svandragt self-assigned this Oct 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants