-
Notifications
You must be signed in to change notification settings - Fork 85
merge 2.x to 3.x #649
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
merge 2.x to 3.x #649
Conversation
Custom HTTP Request Factory | ||
--------------------------- | ||
|
||
The proxy client uses an implementation of ``Http\Message\RequestFactory`` to create HTTP requests. |
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.
I think this is different for FOSHttpCache
v3
(it changed to Psr\Http\Message\RequestFactoryInterface
) but v3
of the bundle supports v2
and v3
of the lib. How do we properly document that?
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.
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.
good point. i actually don't see value in keeping to support FOSHttpCache 2.x with the 3.x bundle. both have the same minimum PHP version, so there is no good argument to use different major versions.
i will adjust accordingly.
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.
ah, and we also added a psr-17 stream factory optional argument. for consistency we should support that as well.
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.
ah, and we also added a psr-17 stream factory optional argument. for consistency we should support that as well.
Should I create a PR for that?
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.
FriendsOfSymfony/FOSHttpCache#595 for improving the phpdoc vor FOSHttpCache 3.x
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.
thanks. i already committed it in this branch :-D
Custom HTTP Request Factory | ||
--------------------------- | ||
|
||
The proxy client uses an implementation of ``Http\Message\RequestFactory`` to create HTTP requests. |
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.
@@ -10,6 +10,7 @@ | |||
public="true"> | |||
<argument type="service" id="fos_http_cache.proxy_client.varnish.http_dispatcher"/> | |||
<argument>%fos_http_cache.proxy_client.varnish.options%</argument> | |||
<argument /> <!-- request factory --> |
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.
We could use a new Symfony 5.1 feature here: https://symfony.com/blog/new-in-symfony-5-1-abstract-service-arguments
I don't know what your strategy with this this.
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.
ah right, that is a thing now. but as null
is a valid argument here, i would keep this as is.
No description provided.