Skip to content

Conversation

@AlexSkrypnyk
Copy link
Member

@AlexSkrypnyk AlexSkrypnyk commented Nov 20, 2025

Summary by CodeRabbit

  • Chores

    • Updated dependency constraints to support PHP_CodeSniffer v4 and Drupal coder v9 (alpha).
  • Tests

    • CI tests expanded to run multiple PHPCS versions and adjusted artifact naming to include PHPCS version.
    • Test runner now invokes PHPCS in quiet mode to reduce output verbosity.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Nov 20, 2025

📝 Walkthrough

Walkthrough

Adds PHPCS v3/v4 testing to the CI matrix, updates composer constraints to allow PHPCS v4 and Drupal coder v9-alpha, and silences PHPCS output in a functional test by adding the -q flag.

Changes

Cohort / File(s) Summary
CI/CD workflow matrix expansion
.github/workflows/test-php.yml
Adds phpcs-versions: ['3','4'] to the job matrix, conditional install steps for PHPCS v3 vs v4 (and matching drupal/coder), and updates artifact upload name to include both php${{ matrix.php-versions }} and phpcs${{ matrix.phpcs-versions }}.
Dependency version constraints
composer.json
Expands squizlabs/php_codesniffer constraint from ^3.10 to `^3.10
Test PHPCS invocation
tests/Functional/FunctionalTestCase.php
Adds the -q (quiet) flag to the PHPCS command invocation used by runPhpcs, reducing PHPCS output verbosity while preserving JSON report parsing.

Sequence Diagram(s)

sequenceDiagram
    participant GH as GitHub Actions runner
    participant Matrix as Job Matrix (php-versions × phpcs-versions)
    participant Step as Job Steps
    participant Artifact as Upload Artifact

    Note over GH,Matrix: For each combination (php, phpcs)
    GH->>Matrix: start job
    Matrix->>Step: checkout, setup php
    Step->>Step: determine phpcs version
    alt phpcs == 3
        Step->>Step: composer require squizlabs/php_codesniffer:^3.10 drupal/coder:^8.3
    else phpcs == 4
        Step->>Step: composer require squizlabs/php_codesniffer:^4 drupal/coder:^9@alpha
    end
    Step->>Step: run tests (PHPCS invoked with -q in functional tests)
    Step->>Artifact: upload `...-php${{ matrix.php-versions }}-phpcs${{ matrix.phpcs-versions }}`
    Artifact-->>GH: artifact stored
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • Review focus: CI conditional steps and artifact naming correctness, composer version-range validity, and the PHPCS -q addition in the test file.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title clearly and directly summarizes the main objective of the changeset: adding PHPCS4 compatibility across workflow configuration, composer dependencies, and test code.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/phpcs4

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f1e42e2 and 11848d9.

📒 Files selected for processing (3)
  • .github/workflows/test-php.yml (3 hunks)
  • composer.json (1 hunks)
  • tests/Functional/FunctionalTestCase.php (1 hunks)
🔇 Additional comments (6)
.github/workflows/test-php.yml (3)

26-26: CI matrix expansion is appropriate for version compatibility testing.

Adding the phpcs-versions matrix doubles the number of CI jobs from 4 to 8, which will increase CI execution time and resource usage. This is a reasonable trade-off to ensure compatibility with both PHPCS v3 and v4 across all supported PHP versions.


50-56: Conditional dependency installation is correctly implemented.

The conditional logic properly pins PHPCS and Drupal Coder versions for each matrix variant using composer require --no-update --with-all-dependencies. This approach correctly modifies the constraints before the final composer install at line 57.


81-81: Artifact naming correctly includes both PHP and PHPCS versions.

The updated artifact name format prevents conflicts between the 8 matrix jobs by including both php-versions and phpcs-versions. This ensures each combination produces a uniquely named artifact.

composer.json (2)

26-26: Drupal Coder v9@alpha is compatible with PHPCS v4 — no changes needed.

Drupal Coder 9.0.0-alpha1 adds support for PHP_CodeSniffer 4.0, and it explicitly requires "squizlabs/php_codesniffer": "^4.0". The @Alpha stability flag is necessary and correctly applied. Since this is a dev dependency validated by CI, the change is sound.


22-22: PHPCS v4 support is properly configured and tested.

Verification confirms the constraint is sound: PHPCS v4.0.1 is stable and the codebase is already prepared for it. Custom sniffs use only public APIs (Sniff interface, File class) that remain compatible with v4. The CI matrix explicitly tests both PHPCS versions across four PHP versions, and external dependencies are correctly versioned: drupal/coder:^8.3 for v3 and drupal/coder:^9@alpha for v4. No breaking changes impact this standard.

tests/Functional/FunctionalTestCase.php (1)

71-71: Good addition: -q flag safely reduces test output noise while preserving JSON report.

The -q (quiet) flag only disables progress/verbose output and does not affect JSON report content. This behavior is consistent across PHPCS 3 and 4, so the JSON parsing logic at line 112 will continue to work as expected. The change appropriately improves automated testing output clarity.


Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov-commenter
Copy link

codecov-commenter commented Nov 20, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (be92eb7) to head (11848d9).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff            @@
##              main        #6   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            3         3           
  Lines          164       164           
=========================================
  Hits           164       164           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@AlexSkrypnyk AlexSkrypnyk enabled auto-merge (rebase) November 20, 2025 08:00
@AlexSkrypnyk AlexSkrypnyk merged commit fb78c59 into main Nov 20, 2025
9 checks passed
@AlexSkrypnyk AlexSkrypnyk deleted the feature/phpcs4 branch November 20, 2025 08:02
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.

3 participants