support php 8.4 #441
support php 8.4 #441
Conversation
|
Hello @nikow13! |
|
Related psalm issue and MR about PHP 8.4 support: |
|
Psalm 6 has been released which supports php 8.4 |
|
ready to go? |
|
looks like php version used for running scrutinizer needs to be bumped |
Yes, it seems that Scrutinizer is managed outside the code, so I can’t update it |
|
We are verifying it :) |
Dockerfile
Outdated
| @@ -1,8 +1,8 @@ | |||
| ARG PHP_VERSION=7.1 | |||
| ARG PHP_VERSION=8.4 | |||
There was a problem hiding this comment.
Why the version 8.4 and not the minor that we can support?
There was a problem hiding this comment.
for local testing with the last version
There was a problem hiding this comment.
that should be the smaller supported version maybe? Like 8.0 🤔
There was a problem hiding this comment.
Talking a bit with @AlessandroMinoccheri we agreed php 7 support can be dropped, but we would like to keep support for php 8.0 on
.github/workflows/build.yml
Outdated
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| php-versions: [ '7.1', '7.2', '7.3', '7.4', '8.0', '8.1', '8.2', '8.3'] | ||
| php-versions: [ '8.2', '8.3', '8.4'] |
There was a problem hiding this comment.
we'd like to keep compatibility from php 8.0
.github/workflows/build.yml
Outdated
| needs: build | ||
| steps: | ||
| - uses: actions/checkout@v3 | ||
|
|
||
| - name: Install PHP | ||
| uses: shivammathur/setup-php@v2 | ||
| with: | ||
| php-version: 7.1 | ||
| tools: composer:v2.2 | ||
| php-version: 8.4 |
There was a problem hiding this comment.
shouldn't we use the smaller supported version (8.0)?
.github/workflows/build.yml
Outdated
| uses: codecov/codecov-action@v1 | ||
| with: | ||
| token: ${{ secrets.CODECOV_TOKEN }} | ||
|
|
||
| phar: | ||
| runs-on: "ubuntu-20.04" | ||
| runs-on: ubuntu-latest |
There was a problem hiding this comment.
would that mean when ubuntu gets upgraded we automatically get the latest version? If that the case an approach where we explicitly set the version is preferred imo
.github/workflows/build.yml
Outdated
| strategy: | ||
| matrix: | ||
| php-versions: [ '7.1', '7.2', '7.3', '7.4', '8.0', '8.1' ] | ||
| coverage-driver: [ 'pcov' ] | ||
| php-versions: [ '8.2', '8.3', '8.4' ] |
Dockerfile
Outdated
| @@ -1,8 +1,8 @@ | |||
| ARG PHP_VERSION=7.1 | |||
| ARG PHP_VERSION=8.4 | |||
There was a problem hiding this comment.
that should be the smaller supported version maybe? Like 8.0 🤔
| @@ -35,7 +35,7 @@ public function shouldMatchNamespacesProvider(): array | |||
| * @param mixed $actualFQCN | |||
| * @param mixed $explanation | |||
| */ | |||
| public function test_it_should_match_namespace_and_descendants($expectedNamespace, $actualFQCN, $explanation): void | |||
| public function test_it_should_match_namespace_and_descendants(string $expectedNamespace, string $actualFQCN, string $explanation): void | |||
There was a problem hiding this comment.
@param in phpdoc does not match with the method signature, would it be possibile to remove it?
| @@ -17,10 +17,10 @@ class TargetPhpVersion | |||
| '8.1', | |||
| '8.2', | |||
| '8.3', | |||
| '8.4', | |||
There was a problem hiding this comment.
iirc here we are defining which version arkitect is capable of parsing. Should we keep versions older than 8.0? if yes, should then keep testing them (at least smoke test the phar) to be sure we are actually able to do so?
| @@ -15,11 +15,9 @@ | |||
| */ | |||
| class ProgressBarProgress implements Progress | |||
| { | |||
| /** @var OutputInterface */ | |||
| private $output; | |||
| private \Symfony\Component\Console\Output\NullOutput|OutputInterface $output; | |||
There was a problem hiding this comment.
isn't NullOutput implementing OutputInterface? If yes, that could be just
private OutputInterface $output;
Hi, |
updates the codebase for PHP 8.4 compatibility and upgrades Psalm to version 6.0. Some errors were introduced with the Psalm update, so I’ve added a baseline for now to prevent build issues