Skip to content

PRESS0-4271 Add PHP syntax check (php -l) to CI workflows for all PHP files#221

Open
vikasrana1998 wants to merge 14 commits into
mainfrom
feature/PRESS0-4271-lint-php-files
Open

PRESS0-4271 Add PHP syntax check (php -l) to CI workflows for all PHP files#221
vikasrana1998 wants to merge 14 commits into
mainfrom
feature/PRESS0-4271-lint-php-files

Conversation

@vikasrana1998

Copy link
Copy Markdown

Proposed changes

Type of Change

Production

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Dependency update
  • Refactoring / housekeeping (changes to files not directly related to functionality)

Development

  • Tests
  • Dependency update
  • Environment update / refactoring
  • Documentation Update

Visual

Checklist

  • I have read the CONTRIBUTING doc
  • I have viewed my change in a web-browser
  • Linting and tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Further comments

@vikasrana1998 vikasrana1998 self-assigned this Apr 22, 2026

Copilot AI 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.

Pull request overview

Adds a reusable GitHub Actions workflow to run php -l against PHP files, enabling PHP syntax validation in CI for repositories that call this workflow.

Changes:

  • Introduces a new reusable workflow (workflow_call) for PHP syntax linting with configurable PHP version.
  • Implements scope logic to lint either PR-changed PHP files (default on PRs) or all PHP files (default otherwise).
  • Adds path-prefix exclusions via a paths-ignore input to avoid linting dependency directories (e.g., vendor, node_modules).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .github/workflows/reusable-php-lint.yml Outdated
Comment thread .github/workflows/reusable-php-lint.yml Outdated
Comment thread .github/workflows/reusable-php-lint.yml Outdated
vikasrana1998 and others added 3 commits May 6, 2026 11:09
- Use git merge-base for changed-files diff so we only lint commits
  introduced by the PR, matching GitHub's "Files changed" view even
  when the base branch has advanced
- Surface git diff failures gracefully: fall back to scope=all with
  a ::notice:: instead of silently producing an empty file list
- Use `set -euo pipefail` in the lint step so unexpected errors
  (e.g. missing FILES_LIST) fail the job rather than reporting success
- Group repeated >> "$GITHUB_OUTPUT" redirects into { ...; } >> file
  to satisfy shellcheck SC2129 and fix the actionlint CI failure

Co-authored-by: Cursor <cursoragent@cursor.com>
Comment thread .github/workflows/example-php-lint.yml Fixed
@vikasrana1998 vikasrana1998 force-pushed the feature/PRESS0-4271-lint-php-files branch from f6c2d00 to 93e85ac Compare May 8, 2026 11:24
arunshenoy99
arunshenoy99 previously approved these changes May 8, 2026
wpscholar
wpscholar previously approved these changes Jun 15, 2026
Comment thread .github/workflows/reusable-php-lint.yml Fixed
Comment thread .github/workflows/reusable-php-lint.yml Fixed
@github-actions

github-actions Bot commented Jun 15, 2026

Copy link
Copy Markdown

AI code review

✅ Strengths

  • Pinned action SHAs and least-privilege permissions are well done (top-level permissions: {}, job-level contents: read) — great security posture.
  • Sensible, composable inputs with clear defaults (php-version, scope auto/changed/all, paths-ignore, timeout) make this easy to adopt across repos.
  • Robust scope resolution with graceful fallbacks: auto → changed when PR SHAs are present, otherwise all; proper fallback to all when SHAs are missing or merge-base/diff fails, with helpful ::notice messages.
  • Good file handling hygiene: safe Bash (set -euo pipefail), mapfile usage, quoting, and explicit filtering/validation of candidate paths.
  • Per-file annotations with parsed line numbers and careful escaping of workflow command values/properties — this will make failures actionable in the UI.
  • Summaries improve DX by showing scope, counts, and an optional file list.
  • Ignores vendor/node_modules by default and re-applies ignore filtering even in changed scope — avoids false positives.

⚠️ Suggestions (non-blocking)

  • Consider parallelizing php -l to speed up large repos, e.g. xargs -P"$(nproc)" php -l, while preserving per-file annotations (you could collect failures and then emit annotations after the parallel run).
  • For the all scope, consider using git ls-files to enumerate tracked PHP files instead of find. This respects sparse checkouts and is often faster and simpler to maintain. Example: git ls-files -z -- ':(icase,glob)**/*.php' | tr '\0' '\n'.
  • Consider supporting additional common PHP-related extensions via an input (e.g., .phtml, .inc, .php5) so callers can opt-in when needed.
  • Optional: cap the “File list” in the step summary (e.g., first 500) and show a count beyond that, to avoid massive summaries on very large repos.
  • Minor readability: the checkout fetch-depth expression could be made more explicit (or a separate step var) to avoid string-vs-number ambiguity and improve clarity for future readers.
  • Minor: when computing changed files you already derive merge_base; using a two-dot diff (merge_base..HEAD) or directly diffing BASE...HEAD without precomputing merge-base would be simpler and less surprising than mixing an explicit merge-base with a triple-dot diff.
  • Minor log noise: after emitting ::error annotations, also echoing the full php -l output duplicates information. Consider keeping the log file + summary but skipping the extra echo to reduce clutter.

❌ Issues (blocking/critical)

  • None identified. The workflow is secure-by-default, robust to common edge cases, and failure modes are well surfaced. Any items above are polish/performance/readability and not merge blockers.

Verdict

⚠️ Verdict: Solid addition that meaningfully improves CI safety for PHP code. Recommend merge with the minor follow-ups above as subsequent polish.


Automation note: This AI review is posted as one regular pull-request conversation comment and will be updated in place on each successful workflow run. It does not create or dismiss formal GitHub Reviews.

@vikasrana1998 vikasrana1998 dismissed stale reviews from wpscholar and arunshenoy99 via cacee7d June 16, 2026 06:11
vikasrana1998 and others added 4 commits June 16, 2026 11:51
- Bump setup-php pin to v2.37.2 to resolve zizmor known-vulnerable-actions
- Match paths-ignore recursively for nested vendor/node_modules directories
- Fix checkout show-progress to use runner.debug boolean directly
Deduplicate and case-insensitively match PHP files, escape workflow
annotations, include php -l error text in annotations, gate verbose file
listing behind debug mode, and add step summary output.

Copilot AI 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.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.

Comment thread .github/workflows/reusable-php-lint.yml
Comment thread .github/workflows/reusable-php-lint.yml
Comment thread .github/workflows/reusable-php-lint.yml
vikasrana1998 and others added 2 commits June 19, 2026 12:36
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
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.

5 participants