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

PHP 8.1 support #9

Merged
merged 26 commits into from
Jan 5, 2022
Merged

PHP 8.1 support #9

merged 26 commits into from
Jan 5, 2022

Conversation

MatyCZ
Copy link

@MatyCZ MatyCZ commented Dec 14, 2021

Q A
Documentation no
Bugfix no
BC Break no
New Feature yes
RFC no
QA no

Description

Initial PHP 8.1 support.

Fixes #8

Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

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

Please check CI failures too

src/DbSelect.php Outdated Show resolved Hide resolved
src/ConfigProvider.php Outdated Show resolved Hide resolved
@froschdesign froschdesign linked an issue Dec 15, 2021 that may be closed by this pull request
@MatyCZ
Copy link
Author

MatyCZ commented Dec 16, 2021

CI failures are caused by wrong type hints in another libraries (laminas-db), different dependencies (CI test on PHP 7.3 fails due to psr/container needs PHP 7.4, ...) or wrong return type of Mock.

Could you please check CI failures? There is probably nothing to fix.

@MatyCZ
Copy link
Author

MatyCZ commented Dec 16, 2021

I created another pull request laminas/laminas-db#236. This will fix psalm failure because of wrong type in Sql\Select::having().

composer.json Outdated Show resolved Hide resolved
composer.json Outdated Show resolved Hide resolved
src/DbSelect.php Outdated Show resolved Hide resolved
src/DbSelect.php Outdated Show resolved Hide resolved
src/DbSelect.php Outdated Show resolved Hide resolved
src/DbSelect.php Outdated Show resolved Hide resolved
src/ConfigProvider.php Outdated Show resolved Hide resolved
@Ocramius
Copy link
Member

A composer update with lowest dependencies (executed on a PHP 7.4 environment) is needed here

@froschdesign
Copy link
Member

@MatyCZ
Please check the results of the code sniffer:

Error: Property \Laminas\Paginator\Adapter\LaminasDb\DbSelect::$sql has useless @var annotation.
Error: Property \Laminas\Paginator\Adapter\LaminasDb\DbSelect::$select has useless @var annotation.
Error: Property \Laminas\Paginator\Adapter\LaminasDb\DbSelect::$countSelect has useless @var annotation.
Error: Property \Laminas\Paginator\Adapter\LaminasDb\DbSelect::$resultSetPrototype has useless @var annotation.
Error: Property \Laminas\Paginator\Adapter\LaminasDb\DbSelect::$rowCount has useless @var annotation.

https://github.com/laminas/laminas-paginator-adapter-laminasdb/pull/9/files#diff-f8dc66f5df7573cebf9d8d594235ad4b1510d2fe3372bf8dd8a05bde4756129dR25

And the Psalm error:

Error: src/DbTableGateway.php:41:29: PossiblyInvalidArgument: Argument 1 of Laminas\Db\Sql\Select::having expects Closure|Laminas\Db\Sql\Where|array<array-key, mixed>|string, possibly different type Closure|Laminas\Db\Sql\Having|non-empty-array<array-key, mixed>|non-falsy-string provided (see https://psalm.dev/092)

https://github.com/laminas/laminas-paginator-adapter-laminasdb/runs/4583362117?check_suite_focus=true#step:3:289

Thanks in advance! 👍

@MatyCZ
Copy link
Author

MatyCZ commented Dec 28, 2021

@froschdesign I am unable to fix psalm error:
#9 (comment)

Matej Szendi and others added 3 commits December 28, 2021 09:10
Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>
Upstream library has an incorrect annotation, which means the correctly marked one here is being incorrectly flagged as invalid.

Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>
@weierophinney
Copy link
Member

@MatyCZ I pushed an update to the Psalm baseline to ignore that error; it's a problem with the upstream laminas-db library, so we can ignore it for now.

However... we cannot accept this patch without signoff. You can add it to your existing commits by following the directions in our contributor document. Once you've done that, ping me, and I'll merge.

Matej Szendi added 10 commits January 5, 2022 09:23
Signed-off-by: Matej Szendi <matej.szendi@bcom.cz>
Signed-off-by: Matej Szendi <matej.szendi@bcom.cz>
Signed-off-by: Matej Szendi <matej.szendi@bcom.cz>
Signed-off-by: Matej Szendi <matej.szendi@bcom.cz>
Signed-off-by: Matej Szendi <matej.szendi@bcom.cz>
Signed-off-by: Matej Szendi <matej.szendi@bcom.cz>
Signed-off-by: Matej Szendi <matej.szendi@bcom.cz>
Signed-off-by: Matej Szendi <matej.szendi@bcom.cz>
Signed-off-by: Matej Szendi <matej.szendi@bcom.cz>
@MatyCZ
Copy link
Author

MatyCZ commented Jan 5, 2022

@weierophinney ping

src/DbSelect.php Outdated Show resolved Hide resolved
src/DbSelect.php Outdated Show resolved Hide resolved
src/DbSelect.php Outdated Show resolved Hide resolved
src/DbSelect.php Outdated Show resolved Hide resolved
src/DbSelect.php Outdated Show resolved Hide resolved
Matej Szendi added 2 commits January 5, 2022 13:21
Signed-off-by: Matej Szendi <matej.szendi@bcom.cz>
Signed-off-by: Matej Szendi <matej.szendi@bcom.cz>
@Ocramius Ocramius self-assigned this Jan 5, 2022
Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @MatyCZ!

@Ocramius Ocramius merged commit b578d92 into laminas:1.1.x Jan 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PHP 8.1 support
5 participants