symfony-cmf / block-bundle

Extends the SonataBlockBundle to integrate with PHPCR ODM
https://cmf.symfony.com
20 stars 51 forks source link

RFC: BlockVarnishCache default TTL configurable by parameter #194

Open gonzalovilaseca opened 10 years ago

gonzalovilaseca commented 10 years ago

What do you think of having the default TTL of BlockVarnishCache configurable by parameter? I need it for my current project, so if it sounds interesting I could do a PR.

dbu commented 10 years ago

you mean this? https://github.com/symfony-cmf/BlockBundle/blob/master/Cache/BlockVarnishCache.php#L117

absolutely would make sense to be configurable. you could change the default to null and if the value is null use the configured default ttl. you could use the DAY constant for the field, btw. https://github.com/sonata-project/cache/blob/master/lib/CacheElement.php#L55

i would use a setDefaultTtl method rather than adding yet more constructor arguments, as there are quite a lot already. then the DI can add a call to that method if the configuration value was set.

gonzalovilaseca commented 10 years ago

Yes exactly, that's what I meant. Sure I was thinking of a setMethod, I'm looking further into this as it seems Sonata has hardcoded that value in several places, so not sure if changing it there would have any effect.

dbu commented 10 years ago

i think here is the right place, as its injected in the constructor of the sonata cache class that is created. sonata cache can't provide a default value there, the factory class here in the bundle can.

lsmith77 commented 10 years ago

ping