-
-
Notifications
You must be signed in to change notification settings - Fork 85
adds support for PHP 8.5 and symfony 8.0 #511
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,33 +10,39 @@ on: | |
|
|
||
| jobs: | ||
| tests: | ||
| name: "Tests" | ||
| runs-on: ${{ matrix.operating-system }} | ||
| name: "Tests ${{ matrix.php-version }} ${{ matrix.dependencies }} (Symfony ${{ matrix.symfony-version }})" | ||
| runs-on: "ubuntu-latest" | ||
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| dependencies: | ||
| - "locked" | ||
| - "lowest" | ||
| - "highest" | ||
| php-version: | ||
| - "8.1" | ||
| - "8.2" | ||
| - "8.3" | ||
| - "8.4" | ||
| operating-system: | ||
| - "ubuntu-latest" | ||
| - "8.5" | ||
| symfony-version: | ||
| - "5.4.*" | ||
| - "6.4.*" | ||
| - "7.3.*" | ||
| - "7.4.*" | ||
| - "8.0.*" | ||
| exclude: | ||
| - php-version: 8.3 | ||
| symfony-version: "8.0.*" | ||
|
|
||
| steps: | ||
| - name: "Checkout" | ||
| uses: "actions/checkout@v4" | ||
| uses: "actions/checkout@v6" | ||
|
|
||
| - name: "Install PHP" | ||
| uses: "shivammathur/setup-php@v2" | ||
| with: | ||
| coverage: "pcov" | ||
| php-version: "${{ matrix.php-version }}" | ||
| ini-values: memory_limit=-1 | ||
| tools: composer:v2 | ||
| tools: flex, composer:v2 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I assume we need symfony flex to download specific symfony version?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that's perfectly fine, I was just making sure I understand the intention 👍 |
||
|
|
||
| - name: "Get Composer Cache Directory" | ||
| id: composer-cache | ||
|
|
@@ -48,9 +54,12 @@ jobs: | |
| with: | ||
| path: | | ||
| ${{ steps.composer-cache.outputs.dir }} | ||
| key: "php-${{ matrix.php-version }}-${{ matrix.dependencies }}-composer-${{ hashFiles('**/composer.lock') }}" | ||
| key: "php-${{ matrix.php-version }}-composer-${{ hashFiles('**/composer.json **/composer.lock') }}" | ||
| restore-keys: | | ||
| php-${{ matrix.php-version }}-${{ matrix.dependencies }}-composer- | ||
| php-${{ matrix.php-version }}-composer- | ||
|
|
||
| - name: "Configure Symfony" | ||
| run: composer config extra.symfony.require "${{ matrix.symfony-version }}" | ||
|
|
||
| - name: "Install lowest dependencies" | ||
| if: ${{ matrix.dependencies == 'lowest' }} | ||
|
|
@@ -60,10 +69,6 @@ jobs: | |
| if: ${{ matrix.dependencies == 'highest' }} | ||
| run: "composer update --no-interaction --no-progress --no-suggest" | ||
|
|
||
| - name: "Install locked dependencies" | ||
| if: ${{ matrix.dependencies == 'locked' }} | ||
| run: "composer install --no-interaction --no-progress --no-suggest" | ||
|
|
||
| - name: "Tests" | ||
| run: "composer test" | ||
|
|
||
|
|
@@ -72,13 +77,13 @@ jobs: | |
| runs-on: "ubuntu-latest" | ||
| steps: | ||
| - name: "Checkout" | ||
| uses: "actions/checkout@v4" | ||
| uses: "actions/checkout@v6" | ||
|
|
||
| - name: "Install PHP" | ||
| uses: "shivammathur/setup-php@v2" | ||
| with: | ||
| coverage: "pcov" | ||
| php-version: "8.1" | ||
| coverage: none | ||
norberttech marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| php-version: "8.3" | ||
| ini-values: memory_limit=-1 | ||
| tools: composer:v2 | ||
|
|
||
|
|
@@ -92,11 +97,11 @@ jobs: | |
| with: | ||
| path: | | ||
| ${{ steps.composer-cache.outputs.dir }} | ||
| key: "php-8.1-locked-composer-${{ hashFiles('**/composer.lock') }}" | ||
| key: "php-8.3-composer-${{ hashFiles('**/composer.json **/composer.lock') }}" | ||
| restore-keys: | | ||
| php-8.1-locked-composer- | ||
| php-8.3-composer- | ||
|
|
||
| - name: "Install locked dependencies" | ||
| - name: "Install dependencies" | ||
| run: "composer install --no-interaction --no-progress --no-suggest" | ||
|
|
||
| - name: "Static Analyze" | ||
|
|
@@ -107,14 +112,14 @@ jobs: | |
| runs-on: "ubuntu-latest" | ||
| steps: | ||
| - name: "Checkout" | ||
| uses: "actions/checkout@v4" | ||
| uses: "actions/checkout@v6" | ||
|
|
||
| - name: "Install PHP" | ||
| uses: "shivammathur/setup-php@v2" | ||
| with: | ||
| coverage: "pcov" | ||
| tools: composer:v2 | ||
| php-version: "8.1" | ||
| php-version: "8.3" | ||
| ini-values: memory_limit=-1 | ||
|
|
||
| - name: "Get Composer Cache Directory" | ||
|
|
@@ -127,11 +132,11 @@ jobs: | |
| with: | ||
| path: | | ||
| ${{ steps.composer-cache.outputs.dir }} | ||
| key: "php-8.1-locked-composer-${{ hashFiles('**/composer.lock') }}" | ||
| key: "php-8.3-composer-${{ hashFiles('**/composer.json **/composer.lock') }}" | ||
| restore-keys: | | ||
| php-8.1-locked-composer- | ||
| php-8.3-composer- | ||
|
|
||
| - name: "Install locked dependencies" | ||
| - name: "Install dependencies" | ||
| run: "composer install --no-interaction --no-progress --no-suggest" | ||
|
|
||
| - name: "Mutation Tests" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,3 +4,4 @@ var/ | |
| .DS_Store | ||
| .idea | ||
| .phpunit.cache/ | ||
| composer.lock | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure about this one, why does it need to be removed?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Without a composer.lock it is easier to test a widen range dependency like I will overhaul the CI to support runs with all supported Symfony Versions on specific PHP Versions.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh I see, now it makes sense. So in general I like this approach, especially that php-matcher doesn't really have other big dependencies. How would you approach this problem if for example there would be another library that we would like to support multiple versions. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,7 +15,7 @@ | |
| } | ||
| ], | ||
| "require": { | ||
| "php": "~8.1 || ~8.2 || ~8.3 || ~8.4", | ||
| "php": "~8.3 || ~8.4 || ~8.5", | ||
| "ext-filter": "*", | ||
| "ext-json": "*", | ||
| "ext-simplexml": "*", | ||
|
|
@@ -24,12 +24,9 @@ | |
| "doctrine/lexer": "^3.0" | ||
| }, | ||
| "require-dev": { | ||
| "phpunit/phpunit": "^10.4", | ||
| "phpunit/phpunit": "^10.5", | ||
| "openlss/lib-array2xml": "^1.0", | ||
| "symfony/expression-language": "^5.4|^6.4|^7.0", | ||
| "symfony/cache": "^5.4|^6.4", | ||
| "nikic/php-parser": "^4.0", | ||
| "symfony/var-exporter": "^5.4|^6.4|^7.0" | ||
|
Comment on lines
-29
to
-32
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I removed those dependencies as they are not used (directly) |
||
| "symfony/expression-language": "^5.4 || ^6.4 || ^7.3 || ^8.0" | ||
| }, | ||
| "suggest": { | ||
| "openlss/lib-array2xml": "In order ot use Coduo\\PHPMatcher\\Matcher\\XmlMatcher", | ||
|
|
@@ -47,10 +44,7 @@ | |
| } | ||
| }, | ||
| "config": { | ||
| "sort-packages": true, | ||
| "platform": { | ||
| "php": "8.1" | ||
| } | ||
| "sort-packages": true | ||
| }, | ||
| "scripts": { | ||
| "benchmark": [ | ||
|
|
@@ -70,26 +64,23 @@ | |
| ], | ||
| "test:mutation": [ | ||
| "Composer\\Config::disableProcessTimeout", | ||
| "tools\/infection\/vendor\/bin\/infection -j2" | ||
| "tools\/infection\/vendor\/bin\/infection --threads=4" | ||
norberttech marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| ], | ||
| "static:analyze": [ | ||
| "tools\/cs-fixer\/vendor\/bin\/php-cs-fixer fix --dry-run", | ||
| "tools\/psalm\/vendor\/bin\/psalm --shepherd --stats", | ||
| "tools\/phpstan\/vendor\/bin\/phpstan analyze -c phpstan.neon" | ||
| ], | ||
| "tools:install": [ | ||
| "composer install --working-dir=./tools/cs-fixer", | ||
| "composer install --working-dir=./tools/infection", | ||
| "composer install --working-dir=./tools/phpbench", | ||
| "composer install --working-dir=./tools/phpstan", | ||
| "composer install --working-dir=./tools/psalm --ignore-platform-req=php" | ||
| "composer install --working-dir=./tools/phpstan" | ||
| ], | ||
| "tools:update": [ | ||
| "composer update --working-dir=./tools/cs-fixer", | ||
| "composer update --working-dir=./tools/infection", | ||
| "composer update --working-dir=./tools/phpbench", | ||
| "composer update --working-dir=./tools/phpstan", | ||
| "composer update --working-dir=./tools/psalm --ignore-platform-req=php" | ||
| "composer update --working-dir=./tools/phpstan" | ||
| ], | ||
| "post-install-cmd": [ | ||
| "@tools:install" | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@norberttech with the following matrix config it can be reduced to only 5 CI jobs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe a run with
dependencies: "locked"is missingThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets keep it as is for now, if it become too long/complicated we can reduce it to what you suggested above, thanks for the example!