Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[FrameworkBundle] Added caching to TemplateController#6083
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
pierredup commentedNov 21, 2012
IMHO I think the caching should be allowed to be set optionally |
kingcrunch commentedNov 21, 2012
I wrote it this way, because I assume, that it will cover more use-cases, than the other way round, but you are right, that this will change the current behaviour. Would like to hear other opinions, because I don't think one uses this action for anything else than fully-static content (means: The current behaviour doesn't feel very useful to me). |
pierredup commentedNov 21, 2012
I totally agree, but I would then suggest keep the caching on by default, but have the option to turn it off if necessary |
pierredup commentedNov 21, 2012
Actually I think to have caching permanently enabled for static content would probably be the best scenario, but I like to think in terms of flexibility and specific user requirements. It would be great to get some opinions from others on this |
kingcrunch commentedNov 23, 2012
I thought about it and I come to the conclusion, that it is probably a not so good idea to enable caching by default, because ... well, it's not possible to disable it again. I guess something like this may be not so uncommon as I suggested in the first commit. |
Changed bevahiour:- No caching by default- Implicit public caching, whe one of maxAge or sharedAge is set- Explicit private caching, when private is set and evaluates to true- Explicit public caching, when private is set and evaluates to false
Removed unused constants
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.
should beBoolean instead ofbool
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.
Just curious:Boolean, but notInteger?
fabpot commentedDec 3, 2012
Can you make a PR for the docs? (symfony/symfony-docs). Thanks. |
Because the main purpose for the
TemplateControllerseems to be to render static pages like "disclaimer" and such, it seems useful to allow caching.