search

infusionsoft / infusionsoft-php

PHP client library for the Infusionsoft API.
https://developer.infusionsoft.com/
Other
130 stars 128 forks source link

PHP 8 Support #279

Closed Ultimater closed 3 years ago

Ultimater commented 3 years ago

I can install using composer install --ignore-platform-reqs and it works on PHP 8. But without that, it won't install on PHP 8, I can only go up to PHP 7.4.

I commented on the package that is holding us back: https://github.com/lstrojny/fxmlrpc/issues/80

georanma commented 3 years ago

@Ultimater I've been waiting on a response in that repo to try the upgrade myself, but I guess we have the answer now.

Ultimater commented 3 years ago

@georanma The response I got was merely the author of the issue claiming it's abandoned. We can be our own judge if a package is abandoned or not. Haven't heard back from Lars Strojny yet that the package is abandoned.

Lars' last comment was 4 months ago:

Haven’t looked at all into support PHP 8.0 yet, so no idea. Can’t imagine it to be terribly hard to achieve so if anybody wants to get a PR rolling, help is very welcome!

The first step would be to run the test suite under PHP 8, that should tell us quite well what is breaking and what works.

He might just need time and/or a reminder to merge the PR and make a release.

Alternatively, we can work on migrating over to a different XMLRPC package.

The older infusionsoft sdk, before OAuth2, made use of the this package: https://github.com/gggeek/phpxmlrpc It has a PHP 8 update. Although I suspect it's losing momentum.

Maybe try out Zend Framework's xmlrpc, which is now Laminas: https://docs.laminas.dev/laminas-xmlrpc/client/

georanma commented 3 years ago

@Ultimater ya, I pulled their package, and started to do the update, but their own internal code sniffs package was a blocker. I didnt feel comfortable working against that.

The Laminas package is probably the way to go, and that it wouldnt take a lot to swap it out. Do you know if the test coverage is good enough on our package, or do we need to do some tests by hand?

Ultimater commented 3 years ago

@georanma The test coverage is very basic. Need to run some tests by hand when doing these kinds of changes.

If you want to tackle it, go for it.

Ultimater commented 3 years ago

https://github.com/lstrojny/fxmlrpc/issues/80 has put out a release version 0.20.0 which adds PHP 8 support.