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

Allow dynamic properties in CurlMultiHandler for PHP 8.2 support #7

Merged
merged 1 commit into from
Oct 25, 2022

Conversation

asgrim
Copy link

@asgrim asgrim commented Sep 15, 2022

Avoids the deprecation notice:

Deprecated: Creation of dynamic property GuzzleHttp\Ring\Client\CurlMultiHandler::$_mh is deprecated in /tmp/scout_elastic_test/vendor/ezimuel/ringphp/src/Client/CurlMultiHandler.php on line 47

Note: this PR does not fix or even check anything else with PHP 8.2 compatibility; just this specific issue.

@@ -16,6 +16,7 @@
*
* @property resource $_mh Internal use only. Lazy loaded multi-handle.
*/
#[AllowDynamicProperties]
Copy link

Choose a reason for hiding this comment

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

Or declare the properties that are used?

Copy link
Author

Choose a reason for hiding this comment

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

I did look at that, but the lazy loading $_mh is a world of pain that would need some significant refactoring 😱

@shyim
Copy link

shyim commented Oct 21, 2022

@ezimuel can we get this merged? It's annoying in the CI 🙈

@ezimuel ezimuel merged commit d13d2a1 into ezimuel:master Oct 25, 2022
@ezimuel
Copy link
Owner

ezimuel commented Oct 25, 2022

Thanks @asgrim for your help.

@ezimuel
Copy link
Owner

ezimuel commented Oct 25, 2022

Just released version 1.2.1 https://github.com/ezimuel/ringphp/releases/tag/1.2.1

@asgrim asgrim deleted the allow-dynamic-properties-curl-mh branch October 26, 2022 07:08
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

Successfully merging this pull request may close these issues.

4 participants