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

Support for PHP 8.4 #160

Closed
amotl opened this issue Nov 21, 2024 · 16 comments
Closed

Support for PHP 8.4 #160

amotl opened this issue Nov 21, 2024 · 16 comments
Assignees
Labels
dependencies Pull requests that update a dependency file help wanted needs: feedback An issue that needs feedback

Comments

@amotl
Copy link
Member

amotl commented Nov 21, 2024

About

PHP 8.4 has been released today.

@amotl
Copy link
Member Author

amotl commented Feb 9, 2025

GH-163 verifies support for PHP 8.4. No changes were needed.

@michabbb
Copy link
Contributor

michabbb commented Feb 10, 2025

@amotl can you please make a new up-to-date release? the last is from 2023. thanks!

@amotl amotl self-assigned this Feb 11, 2025
@amotl
Copy link
Member Author

amotl commented Feb 12, 2025

crate-pdo 2.2.1 has been released. Can you check if this works well for you, @michabbb?

@amotl amotl added the dependencies Pull requests that update a dependency file label Feb 12, 2025
@michabbb
Copy link
Contributor

i was forced to make these changes to avoid some deprecation warnings: #164
till now I only tried to select data, maybe more pieces need a fix.

@amotl
Copy link
Member Author

amotl commented Feb 12, 2025

Thanks a stack for your support! 💯

@amotl amotl added help wanted needs: feedback An issue that needs feedback labels Feb 12, 2025
@michabbb
Copy link
Contributor

i tested your version 2.2.1, with php 8.4.3 i´m getting these warnings:

Deprecated: Crate\PDO\PDOStatementImplementationPhp8::fetchAll(): Implicitly marking parameter $mode as nullable is deprecated, the explicit nullable type must be used instead in /app/vendor/crate/crate-pdo/src/Crate/PDO/PDOStatementImplementationPhp8.php on line 54

Deprecated: Return type of Crate\PDO\PDOStatementImplementation::setFetchMode(int $mode, ...$args) should either be compatible with PDOStatement::setFetchMode(int $mode, mixed ...$args): true, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in /app/vendor/crate/crate-pdo/src/Crate/PDO/PDOStatementImplementationPhp8.php on line 40

that´s why I made that changes on my side 🤷

@amotl
Copy link
Member Author

amotl commented Feb 12, 2025

Thanks for your report. I am observing more deprecation warnings, yet the software tests succeed, so I thought we can address them on subsequent iterations. Are those deprecation warnings causing more serious downstream issues, so that crate-pdo version 2.2.1 is effectively useless?

php -v
PHP 8.4.3 (cli) (built: Jan 15 2025 01:03:17) (NTS)
Copyright (c) The PHP Group
Built by Homebrew
Zend Engine v4.4.3, Copyright (c) Zend Technologies
    with Zend OPcache v8.4.3, Copyright (c), by Zend Technologies
composer run test
Deprecated: Crate\PDO\Exception\UnsupportedException::__construct(): Implicitly marking parameter $previous as nullable is deprecated, the explicit nullable type must be used instead in /Users/amo/dev/crate/drivers/crate-pdo/src/Crate/PDO/Exception/UnsupportedException.php on line 31
Deprecated: Crate\PDO\Http\ServerPool::__construct(): Implicitly marking parameter $client as nullable is deprecated, the explicit nullable type must be used instead in /Users/amo/dev/crate/drivers/crate-pdo/src/Crate/PDO/Http/ServerPool.php on line 75
Deprecated: The API interface `Crate\PDO\PDO` is deprecated and will be removed on one of the upcoming releases. Please use `Crate\PDO\PDOCrateDB` instead. in /Users/amo/dev/crate/drivers/crate-pdo/src/Crate/PDO/PDO.php on line 37
Deprecated: Crate\PDO\Exception\InvalidArgumentException::__construct(): Implicitly marking parameter $previous as nullable is deprecated, the explicit nullable type must be used instead in /Users/amo/dev/crate/drivers/crate-pdo/src/Crate/PDO/Exception/InvalidArgumentException.php on line 29

@michabbb
Copy link
Contributor

I wouldn't say useless, but for someone who is somewhat perfectionistic and doesn't want to do things halfway while wanting to use PHP 8.4, there's no way around adjusting a few things. I don't care if some changes don't fit somewhere between 8.0 and 8.4 because I'm only working with >=8.4 here – you, as the maintainer, of course, have the need to keep things compatible with older PHP 8 versions, which I understand and appreciate 😏

@michabbb
Copy link
Contributor

und btw... servus aus bayern nach bayern 😊 🍻

@amotl
Copy link
Member Author

amotl commented Feb 12, 2025

I am just trying to find out if those warnings have been hard errors for you, or if it's merely about a cosmetic issue.
On the latter, I totally hear you: GH-167 intends to also fix the other deprecation warnings.

@amotl
Copy link
Member Author

amotl commented Feb 12, 2025

crate-pdo 2.2.2 has been released, including your improvements. Can you check if this works better for you now, @michabbb?

@michabbb
Copy link
Contributor

i am working with laravel, so seeing these ugly notices all the time, pardon, that makes me crazy 🙈
these notices are cosmetic, sure.. but I want everything "perfect" 😏
so don´t worry. i can perfectly work with my fork... just because of me, there is no need to fix something here and now.
okay will test 2.2.2....

@michabbb
Copy link
Contributor

michabbb commented Feb 12, 2025

yep, works.. no more deprecation warnings 👍 🥳

@amotl
Copy link
Member Author

amotl commented Feb 12, 2025

It works. No more deprecation warnings.

Excellent, thanks.

I am working with Laravel [...]

How do you handle this situation? Do you also need an updated Doctrine/DBAL driver?

NB: Please add your comments over there when applicable.

@michabbb
Copy link
Contributor

i am using laravel 11.42.1 (latest version) and i am able to connect to crate and get data like this:

$connection = app(CrateDB::class)->createConnection();
$stmt       = $connection->query('SELECT id FROM something LIMIT 3');
$rows       = $stmt->doFetchAll(PDO::FETCH_ASSOC);

and everything is working fine 🤷 😏

@amotl
Copy link
Member Author

amotl commented Feb 17, 2025

Thank you very much. I think it is safe to close the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file help wanted needs: feedback An issue that needs feedback
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants