btcpayserver / magento2-plugin

4 stars 5 forks source link

PHP 8.1 deprecation warnings in Result\AbstractResult class #13

Closed Jacquesvw closed 1 year ago

Jacquesvw commented 1 year ago

I'm testing the magento2-plugin 2.0.1 on magento 2.4.5-p1 running php8.1.13 using MAMP and experiencing the Result\AbstractResult deprecation warnings that were fixed in the btcpayserver-greenfield-php 2.0.0 update.

Would it be possible to update the plugin to use btcpayserver-greenfield-php 2.1.0 to add support for PHP8.1?

woutersamaey commented 1 year ago

Sure, but updating the package is not enough as there are some breaking changes. I'll look into it. Shouldn't be a lot of work.

Jacquesvw commented 1 year ago

Thank you. Appreciate it.

woutersamaey commented 1 year ago

Turns out it was less work that I expected. I haven't tested yet, but if you're willing to be my guinea pig, please update to the current master latest commit and try it out? If it works, I'll release as a new version for everyone.

Jacquesvw commented 1 year ago

Thank you. I installed the #master and ran into the following error regarding btcpayserver-greenfield-php:

Fatal error: During inheritance of Countable: Uncaught Exception: Deprecated Functionality: Return type of BTCPayServer\Result\AbstractListResult::count() should either be compatible with Countable::count(): int, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in /******/magento/vendor/btcpayserver/btcpayserver-greenfield-php/src/Result/AbstractListResult.php on line 9

I temporarily suppressed the error by adding #[\ReturnTypeWillChange] to line 9 before public function count() and now magento2-plugin is working. The error above is a btcpayserver-greenfield-php issue with php8.1, but I did not find other problems with the magento2-plugin.

woutersamaey commented 1 year ago

@ndeet your thoughts?

Jacquesvw commented 1 year ago

Fix is to add the return type to line 9: public function count() -> public function count(): int in file: btcpayserver-greenfield-php/src/Result/AbstractListResult.php

ndeet commented 1 year ago

Hmm, interesting, I thought I remember we already fixed that or something similar, also strange that it had not had the return type in the first place already. Seems we did not notice it was missing. Is a non breaking change and I can do a release after it got merged.

https://github.com/btcpayserver/btcpayserver-greenfield-php/pull/106

ndeet commented 1 year ago

Fyi, release is out https://github.com/btcpayserver/btcpayserver-greenfield-php/releases/tag/v2.2.0

woutersamaey commented 1 year ago

I bumped the version and released 2.0.2 of the Magento 2 plugin so @Jacquesvw no longer needs to track master. Everything OK now?

Jacquesvw commented 1 year ago

Thank you @woutersamaey

I can confirm that everything is working. Appreciate your assistance with this.