Make 8.5 the main PHP version#249
Conversation
Pull Request Test Coverage Report for Build 20537035017Details
💛 - Coveralls |
There was a problem hiding this comment.
Pull request overview
This pull request attempts to upgrade the project's main PHP version from 8.4 to 8.5 and adds transpilation support to PHP 8.4. However, there is a critical issue: PHP 8.5 does not exist yet. As of January 2026, PHP 8.4 is the latest stable version (released November 2024). The PR also updates test code to conditionally call setAccessible() on reflection properties only for PHP versions before 8.1, since this method became a no-op in PHP 8.1.
Key Changes:
- Updates PHP requirement from ^8.4 to ^8.5 across configuration files
- Adds conditional checks for
setAccessible()calls in test files (only called for PHP < 8.1) - Adds new rector.84.php configuration for transpiling to PHP 8.4
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| composer.json | Updates PHP requirement to ^8.5 (version does not exist) |
| rector.php | Adds SetList::PHP_85 to configuration (constant may not exist) |
| rector.84.php | New file adding PHP 8.4 downgrade configuration (constant may not exist) |
| tests/UnleashBuilderTest.php | Adds PHP_VERSION_ID checks before setAccessible() calls throughout test methods |
| tests/Helper/NetworkCalculatorTest.php | Adds PHP_VERSION_ID checks before setAccessible() calls in helper methods |
| .github/workflows/transpile.yaml | Updates PHP version to 8.5 and adds 8.4 to transpilation matrix |
| .github/workflows/tests.yaml | Updates PHP version references to 8.5 and adds 8.4 to test matrix |
| .github/workflows/tests-8.x.yaml | Adds 8.4 to the PHP version test matrix |
| .github/workflows/release.yaml | Updates PHP version to 8.5, changes release tags to v${VERSION}85, adds 8.4 to matrix |
| .github/workflows/code-coverage.yml | Updates PHP version to 8.5 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| run: composer require symfony/polyfill-php73:^1.0 | ||
| - name: Update composer.json version | ||
| run: 'sed -i -e ''s/"php": "\^8.4"/"php": "\^${{ matrix.version }}"/'' composer.json' | ||
| run: 'sed -i -e ''s/"php": "\^8.5"/"php": "\^${{ matrix.version }}"/'' composer.json' |
There was a problem hiding this comment.
This sed pattern replacement expects the composer.json to contain "php": "^8.5", but since PHP 8.5 doesn't exist, this will cause issues. The pattern should match the actual PHP version in composer.json.
| run: composer require symfony/polyfill-php73:^1.0 | ||
| - name: Update composer.json version | ||
| run: 'sed -i -e ''s/"php": "\^8.4"/"php": "\^${{ matrix.version }}"/'' composer.json' | ||
| run: 'sed -i -e ''s/"php": "\^8.5"/"php": "\^${{ matrix.version }}"/'' composer.json' |
There was a problem hiding this comment.
This sed pattern replacement expects the composer.json to contain "php": "^8.5", but since PHP 8.5 doesn't exist, this will cause issues. The pattern should match the actual PHP version in composer.json.
| run: composer require symfony/polyfill-php73:^1.0 | ||
| - name: Update composer.json version | ||
| run: 'sed -i -e ''s/"php": "\^8.4"/"php": "\^${{ matrix.version }}"/'' composer.json' | ||
| run: 'sed -i -e ''s/"php": "\^8.5"/"php": "\^${{ matrix.version }}"/'' composer.json' |
There was a problem hiding this comment.
This sed pattern replacement expects the composer.json to contain "php": "^8.5", but since PHP 8.5 doesn't exist, this will cause issues. The pattern should match the actual PHP version in composer.json.
| run: 'sed -i -e ''s/"php": "\^8.5"/"php": "\^${{ matrix.version }}"/'' composer.json' | |
| run: 'sed -i -e ''s/"php": "[^"]*"/"php": "\^${{ matrix.version }}"/'' composer.json' |
| if (PHP_VERSION_ID < 80100) { | ||
| $strategiesProperty->setAccessible(true); | ||
| } |
There was a problem hiding this comment.
A helper method 'getProperty' already exists at line 1139 that encapsulates the PHP_VERSION_ID check for setAccessible. Consider refactoring the repeated inline version checks throughout this file to use this existing helper method instead, which would reduce code duplication and improve maintainability.
RikudouSage
left a comment
RikudouSage
left a comment
There was a problem hiding this comment.
LGTM. I didn't check whether all setAccessible() calls are updated, I'm gonna assume they are.
@RikudouSage Yes, I added a PHP version check to all of them. |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Description
setAccessible()calls which are noop since PHP 8.1.Checklist: