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

Upgrade Guzzle Promises Function API to Static API #104

Merged
merged 2 commits into from
Jun 16, 2023

Conversation

DimionX
Copy link
Contributor

@DimionX DimionX commented Jun 7, 2023

Guzzle Promises:
A static API was first introduced in 1.4.0, in order to mitigate problems with functions conflicting between global and local copies of the package. The function API was removed in 2.0.0.

https://github.com/guzzle/promises#upgrading-from-function-api

Guzzle Promises:
A static API was first introduced in 1.4.0, in order to mitigate problems with functions conflicting between global and local copies of the package. The function API was removed in 2.0.0.

https://github.com/guzzle/promises#upgrading-from-function-api
@coveralls
Copy link

coveralls commented Jun 12, 2023

Coverage Status

coverage: 91.479% (+91.5%) from 0.0% when pulling 2b7d59b on DimionX:patch-1 into add74da on ackintosh:master.

@ackintosh
Copy link
Owner

@DimionX Thank you for this PR!


The tests with --prefer-lowest have failed with the error:

https://github.com/ackintosh/ganesha/actions/runs/5200076716/jobs/9482744646?pr=104

  1. Ackintosh\Ganesha\GuzzleMiddlewareTest::reject
    Failed asserting that exception of type "Error" matches expected exception "Ackintosh\Ganesha\Exception\RejectedException". Message was: "Class "GuzzleHttp\Promise\Create" not found"

This tells us that we should support the both promise_for and Create::promiseFor. I think it has been done with the code like:

if (class_exists('\GuzzleHttp\Promise\Create')) {
    return call_user_func('\GuzzleHttp\Promise\Create::rejectionFor', ...);
} else {
    return call_user_func('\GuzzleHttp\Promise\rejection_for', ...);
}

(this is just for example, there may exist a cleaner way)

@ackintosh
Copy link
Owner

I will add commits to this Pull Request to fix the error with --prefer-lowest. Please give me a little time.

@ackintosh
Copy link
Owner

@DimionX I have updated your Pull Request to fix an error detected by CI, now the tests on CI have passed. Could you try the updated one to check if the problem you reported has been fixed on your environment?

@DimionX
Copy link
Contributor Author

DimionX commented Jun 16, 2023

@ackintosh Thanks! The problem is solved, now everything is fine

@ackintosh ackintosh merged commit a4c08dc into ackintosh:master Jun 16, 2023
@ackintosh
Copy link
Owner

@DimionX This PR has been released as v3.1.1. 🎉 Thank you for your contribution!
https://github.com/ackintosh/ganesha/releases/tag/3.1.1

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.

3 participants