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 7 deprecation errors #32

Closed
andreasnij opened this issue Jan 19, 2016 · 12 comments
Closed

PHP 7 deprecation errors #32

andreasnij opened this issue Jan 19, 2016 · 12 comments

Comments

@andreasnij
Copy link

Several classes in this library triggers deprecation errors with PHP 7. Example:

Methods with the same name as their class will not be constructors in a future version of PHP; 
xmlrpc_client has a deprecated constructor

Easy to fix by changing the constructor name to __construct.

Hoping for a quick patch release. Thanks!

@gggeek
Copy link
Owner

gggeek commented Jan 19, 2016

Hi. Did you try out the version 4.0.0-alpha release? That will be the only one supported going forward

@HMAZonderland
Copy link
Contributor

Is it possible to tag this as normal release? Or is the refresh of the lib not finished?

@gggeek
Copy link
Owner

gggeek commented Jan 19, 2016

I had a couple of things I still wanted to do before tagging it as stable, but nothing critical, so I guess they can go into 4.1 (or 4.0.1 in case of regressions ;-) ). Will try to push out the tag by end of week...

@andreasnij
Copy link
Author

The 4.0 release looks much better but I'm afraid that doesn't help us at the moment. We are using another library which has this library as a dependency , so I'm not in control of the code. I also understand that 4.0 comes with some API changes, so I don't expect them to upgrade to 4.0 anytime soon. A small 3.0.2 patch release would solve the problem though (since they have ("phpxmlrpc/phpxmlrpc": "~3.0" in their composer file).

I can submit a pull request with the changes if want.

@gggeek
Copy link
Owner

gggeek commented Jan 19, 2016

@jandreasn I see.

Tbh, I am a bit loath to introduce any change to version 3, even if at the moment it works perfectly well with all php versions up to 5.X.
It is true that version 4 has a big API change, but it also has a very thoroughly tested api-compatibility layer, which has been designed to provide drop-in replacement for all users stuck on v3. In fact, the whole testsuite runs on the compatibility layer, not on the new API :-)

And the v3 has to be deprecated at "some" point in time. Heck, I have been "maintaining" it for over 10 years now, and when I look at that code I'm not the proudest of coders. The release of php7 is a very good opportunity to motivate users to go over the small hump that the lib upgrade represents (I am sure in the rest of the codebase of your app there are many fixes to apply...).

If your dependency is already managed via composer, I'd think that bumping up the composer.json to require ~4.0 would not be too hard?

@andreasnij
Copy link
Author

I do understand your reluctance to make changes in a legacy code base. But since this library already requires PHP 5 (stated in the composer file), and __construct has been the recommended constructor syntax since PHP 5.0, I think it is a pretty straightforward thing to change with minimal risk.

I will try to make the other library maintainer migrate to phpxmlrpc 4.0 once it is released as stable, but I think the chances of that are very slim or at least will take a very long time (it's a big company).

@HMAZonderland
Copy link
Contributor

If I remember correctly phpxmlrpc 4 is backwards compatible with phpxmlrpc 3?

@gggeek
Copy link
Owner

gggeek commented Jan 20, 2016

btw: version 4.0.0 released :-)

@HMAZonderland
Copy link
Contributor

Nice work! Good refactoring and future proof for a while.

@Flyingmana
Copy link

I would still like to have this change into the 3.x version.
We currently have a module which is making use of it, and is now breaking for PHP7. The problem is, our setup is currently not ready to run on composer and it is also not compatible with the Composer Autoloader. Its also not trivial to add another autoloader(I saw the version 4.x contains one).

It would have spared us from now investing quite some work into doing the renaming ourself.
But would you at least accept a PR, if we provide one? So others dont need to do the same annoying manual work as we did.

@gggeek
Copy link
Owner

gggeek commented Jun 14, 2017

Ok, by popular request, let's get the ball rolling with a PR...

@gggeek
Copy link
Owner

gggeek commented Jul 1, 2017

Version 3.1.0 released, which should fix any problems with php 7.x.

Please open new tickets if you find that not to be the case

@gggeek gggeek closed this as completed Jul 1, 2017
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

No branches or pull requests

4 participants