Skip to content
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

feat(dx): use matrix to test php 8.2+ #353

Closed
wants to merge 1 commit into from

Conversation

petski
Copy link

@petski petski commented Nov 26, 2024

No description provided.

@samsonasik
Copy link
Member

The code is downgraded to php 7.4 on target rector/rector, so using multiple php versions is not needed for CI imo.

@petski
Copy link
Author

petski commented Nov 26, 2024

Thanks @samsonasik, didn't knew and explains a lot :).

Off topic:

I was actually boyscouting while trying to investigate PHP Deprecated: Rector\Doctrine\NodeManipulator\ColumnPropertyTypeResolver::__construct(): Implicitly marking parameter $doctrineTypeToScalarType as nullable is deprecated, the explicit nullable type must be used instead in /.../vendor/rector/rector/vendor/rector/rector-doctrine/src/NodeManipulator/ColumnPropertyTypeResolver.php on line 54 we got in PHP 8.4.1.

The following patch (in the downgraded code!) would resolve this:

- public function __construct(PhpDocInfoFactory $phpDocInfoFactory, TypeFactory $typeFactory, AttributeFinder $attributeFinder, array $doctrineTypeToScalarType = null)
+ public function __construct(PhpDocInfoFactory $phpDocInfoFactory, TypeFactory $typeFactory, AttributeFinder $attributeFinder, array|null $doctrineTypeToScalarType = null)

But as union types are not available until 8.0, that would break stuff for people using PHP 7.4.

If you can point me in the right direction, I'd be happy to supply a patch for this in a new PR/issue.

@TomasVotruba
Copy link
Member

Closing then 👍

@samsonasik
Copy link
Member

samsonasik commented Nov 26, 2024

@petski that resolved at PR:

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