Skip to content

Allow installing in PHP 8.2 #5

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
AlexSkrypnyk merged 1 commit into from
Nov 18, 2025
Merged

Allow installing in PHP 8.2 #5

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 15 additions & 5 deletions .github/workflows/test-php.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ jobs:

strategy:
matrix:
php-versions: ['8.3', '8.4', '8.5']
php-versions: ['8.2', '8.3', '8.4', '8.5']

steps:
- name: Checkout code
Expand All @@ -42,7 +42,11 @@ jobs:
ini-values: pcov.directory=.

- name: Install dependencies
run: composer install ${{ matrix.php-versions == '8.5' && '--ignore-platform-reqs' || '' }}
run: |
if [ "${{ matrix.php-versions }}" == "8.2" ]; then
composer require --dev phpunit/phpunit:^11 --no-update
fi
composer install ${{ matrix.php-versions == '8.5' && '--ignore-platform-reqs' || '' }}
Comment on lines 44 to +49
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Nov 18, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Add clarifying comment explaining the PHP 8.2 + PHPUnit version handling.

The conditional PHPUnit 11 requirement for PHP 8.2 is necessary because PHPUnit 12 requires PHP 8.3+, while PHPUnit 11 requires PHP 8.2+. The implementation is correct, but a comment explaining this constraint mismatch would help future maintainers understand why the workaround is needed.

Consider adding an inline comment like this:

      - name: Install dependencies
        run: |
+         # PHPUnit 12 requires PHP 8.3+, so we downgrade to PHPUnit 11 for PHP 8.2 testing
          if [ "${{ matrix.php-versions }}" == "8.2" ]; then
            composer require --dev phpunit/phpunit:^11 --no-update
          fi
          composer install ${{ matrix.php-versions == '8.5' && '--ignore-platform-reqs' || '' }}

This makes the version constraint rationale clear at a glance.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Install dependencies
run: composer install ${{ matrix.php-versions == '8.5' && '--ignore-platform-reqs' || '' }}
run: |
if [ "${{ matrix.php-versions }}" == "8.2" ]; then
composer require --dev phpunit/phpunit:^11 --no-update
fi
composer install ${{ matrix.php-versions == '8.5' && '--ignore-platform-reqs' || '' }}
- name: Install dependencies
run: |
# PHPUnit 12 requires PHP 8.3+, so we downgrade to PHPUnit 11 for PHP 8.2 testing
if [ "${{ matrix.php-versions }}" == "8.2" ]; then
composer require --dev phpunit/phpunit:^11 --no-update
fi
composer install ${{ matrix.php-versions == '8.5' && '--ignore-platform-reqs' || '' }}
🤖 Prompt for AI Agents 
In .github/workflows/test-php.yml around lines 44 to 49, add a brief inline
comment above the conditional composer require line explaining that PHPUnit 12
requires PHP 8.3+, so for the PHP 8.2 matrix entry we explicitly require
phpunit/phpunit:^11 to ensure compatibility; optionally also add a short note on
the following composer install line about using --ignore-platform-reqs for the
PHP 8.5 job to clarify that it bypasses platform checks.


- name: Validate composer.json
run: |
Expand All @@ -54,11 +58,17 @@ jobs:
continue-on-error: ${{ vars.CI_LINT_IGNORE_FAILURE == '1' }}

- name: Run tests
run: composer test-coverage
run: |
if [ "${{ matrix.php-versions }}" != "8.2" ]; then
composer test-coverage
else
composer test
fi
Comment on lines 60 to +66
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Nov 18, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Clarify rationale for skipping coverage on PHP 8.2.

The conditional logic skips coverage collection for PHP 8.2. Is this intentional to reduce CI time, or is there a technical limitation with PHPUnit 11 or the coverage tooling?

Given that Codecov reports 100% coverage and all tests pass, this might be acceptable. However, documenting the rationale would help future maintainers understand this decision.

Consider adding a comment explaining why coverage is skipped for PHP 8.2:

      - name: Run tests
        run: |
+         # Skip coverage for PHP 8.2 to reduce CI time (coverage is collected on other PHP versions)
          if [ "${{ matrix.php-versions }}" != "8.2" ]; then
            composer test-coverage
          else
            composer test
          fi
        continue-on-error: ${{ vars.CI_TEST_IGNORE_FAILURE == '1' }}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Run tests
run: composer test-coverage
run: |
if [ "${{ matrix.php-versions }}" != "8.2" ]; then
composer test-coverage
else
composer test
fi
- name: Run tests
run: |
# Skip coverage for PHP 8.2 to reduce CI time (coverage is collected on other PHP versions)
if [ "${{ matrix.php-versions }}" != "8.2" ]; then
composer test-coverage
else
composer test
fi
🤖 Prompt for AI Agents 
.github/workflows/test-php.yml lines 60-66: the workflow conditionally skips
coverage for PHP 8.2 but has no explanation; add a brief inline comment above or
beside the conditional explaining the rationale (e.g., to reduce CI time on the
latest matrix entry, or due to a known PHPUnit 11/coverage-tooling
incompatibility) and, if applicable, include a link to an issue or ticket that
documents the technical limitation and note whether skipping is temporary.

continue-on-error: ${{ vars.CI_TEST_IGNORE_FAILURE == '1' }}

- name: Upload coverage report as an artifact
uses: actions/upload-artifact@v5
if: ${{ matrix.php-versions != '8.2' }}
with:
name: ${{github.job}}-code-coverage-report-${{ matrix.php-versions }}
path: .logs
Expand All @@ -67,7 +77,7 @@ jobs:

- name: Upload test results to Codecov
uses: codecov/test-results-action@v1
if: ${{ env.CODECOV_TOKEN != '' }}
if: ${{ env.CODECOV_TOKEN != '' && matrix.php-versions != '8.2' }}
with:
files: .logs/junit.xml
fail_ci_if_error: true
Expand All @@ -76,7 +86,7 @@ jobs:

- name: Upload coverage report to Codecov
uses: codecov/codecov-action@v5
if: ${{ env.CODECOV_TOKEN != '' }}
if: ${{ env.CODECOV_TOKEN != '' && matrix.php-versions != '8.2' }}
with:
files: .logs/cobertura.xml
fail_ci_if_error: true
Expand Down
2 changes: 1 addition & 1 deletion composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"source": "https://github.com/drevops/phpcs-standard"
},
"require": {
"php": ">=8.3",
"php": ">=8.2",
"dealerdirect/phpcodesniffer-composer-installer": "^1",
"squizlabs/php_codesniffer": "^3.10"
},
Expand Down
2 changes: 1 addition & 1 deletion src/DrevOps/Sniffs/NamingConventions/LocalVariableSnakeCaseSniff.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ final class LocalVariableSnakeCaseSniff extends AbstractSnakeCaseSniff {
/**
* Error code for non-snake_case variables.
*/
public const string CODE_VARIABLE_NOT_SNAKE_CASE = 'NotSnakeCase';
public const CODE_VARIABLE_NOT_SNAKE_CASE = 'NotSnakeCase';

/**
* {@inheritdoc}
Expand Down
2 changes: 1 addition & 1 deletion src/DrevOps/Sniffs/NamingConventions/ParameterSnakeCaseSniff.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ final class ParameterSnakeCaseSniff extends AbstractSnakeCaseSniff {
/**
* Error code for non-snake_case parameters.
*/
public const string CODE_PARAMETER_NOT_SNAKE_CASE = 'NotSnakeCase';
public const CODE_PARAMETER_NOT_SNAKE_CASE = 'NotSnakeCase';

/**
* {@inheritdoc}
Expand Down