-
Notifications
You must be signed in to change notification settings - Fork 1
Add result object that holds response and has helper methods #2
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
base: master
Are you sure you want to change the base?
Conversation
|
You are trying to make a lot of changes in one go. It's not ok. |
|
Hello @osavchenko, Please see this just as a start of a WIP PR. It is not ready yet! Thanks for your feedback! I could rebase it, and crunch it into smaller commits. On the other side rebase -i has to be done anyway, when this is ready. Just let me know, how I should split the big commit. The main intend of the new result Class was that I am still thinking about the response object. ATM I have no real personal usecase for wrapping it into the result object. It was just inspired from elasticsearch client lib "elastica": It returns an ResultSet with In the end it would be nice to have a "real" object with some methods like I dont know, which direction (if any) are you going with this Bundle? Thank you for your feedback! 👍 |
| environment: | ||
| php: | ||
| version: 7.0 | ||
| version: 7.0.8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, revert this change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. It was just a blind guess. Scruntinizer still does not work.
|
I suggest to split this PR to 2 ones:
Also, I think about caching responses (because openweathermap has own limits for requests) and store cached result into database with ORM or ODM. About helper methods: |
I will do so.
Shouldn't caching be some of the regular cache? Like memcached, redis, filesystem.. ? I like the idea of (optional) caching!
ACK - Symfony Serializer will get nice Annotation support like JMS already has in v3.2 btw. |
What do you mean?
I also like JMSSerializer :) |
I meant that an ORM is not an cache. Maybe the lib could implement the PSR-6 Caching Standard and let the user decide what to use? |
This PR adds a result object that wraps the result object.
The result object has two helper methods:
->asArray()and->asJson()and delegates the property access via magic call to the wrapped result.