Conversation
|
We need that in this package, indeed. Looks good to me. |
|
+1 |
There was a problem hiding this comment.
Well. Maybe looks good. but doesn't work
For instance, the "fos_http_cache.tags.header" parameter is never set based on the yml config, so #25 does not work even with this PR applied ( I have to explicitly set the fos_http_cache.tags.header parameter )
|
Ok, was afraid of that, then we need a pass or custom config for this ala what fos does for tag handler (but on 1.x tag handler seems to only work if varnish is configured, so I think we need something abstract of this): https://github.com/FriendsOfSymfony/FOSHttpCacheBundle/blob/master/src/DependencyInjection/FOSHttpCacheExtension.php#L401 |
|
@vidarl Checked this and this PR works as intended, debugging config shows they have been set. What is wrong is that in #25 I assumed this would be available as param as well, it's not, so I need to do a pass there to extract this. There is Hence the updated Warning inline here.. Merging this and continuing in #25 to get the right value out. |
In the need for specifying tags.header config, I copied over logic from CoreBundle.
@bdunogier Is this ok approach? I would assume we need it here anyway once stuff like this is removed from kernel. Order might be open question here tough, this bundle is registered before kernel,. so kernel might overwrite the values it does have (all but tags section) if we attempt to change them here (but we don't, so for now it's fine afaik).