config.action_controller.perform_caching isn't used anymore in asset pipeline #2889

Merged
merged 1 commit into from Sep 6, 2011

Projects

None yet

6 participants

@guilleiguaran
Member

config.action_controller.perform_caching isn't used anymore in assets pipeline, instead we are using config.assets.digest now

@spastorino
Member

@guilleiguaran we need a test case

@josevalim
Member

we probably already have a test case. we just need to change its configs.

@guilleiguaran guilleiguaran config.action_controller.perform_caching isn't used anymore in assets…
… pipeline, instead we are using config.assets.digest now
6fc518e
@guilleiguaran
Member

I added a test for this part of code, when digest = true Rails.application.assets must be a Sprockets::Index instead of Sprockets::Environment

@spastorino spastorino merged commit 1e61f26 into rails:master Sep 6, 2011
@spohlenz
Contributor

I think action_controller.perform_caching was correct in this situation.

The Sprockets index is related to caching, and doesn't really have anything to do with digests.

Member

@spohlenz, we added the config.assets.digest just to remove all the references of config.action_controller.perform_caching in asset pipeline:

f443f9c#L1R142
f443f9c#L1L159

Contributor

I'm not sure if removing it entirely is all that necessary IMHO.

The other changes were reasonable because the perform_caching flag was used to control whether digests were added or not. However they're two separate functions and the appropriate configuration variables should be used for each.

Member
josh replied Sep 7, 2011

perform_caching still makes sense here

Member

We should stop using perform_caching for everything. We should have decoupled options. Those decouples options could have their default value depending on perform_caching, but we do need to have decoupled options.

Member

We should stop using perform_caching for everything. We should have decoupled options. Those decouples options could have their default value depending on perform_caching, but we do need to have decoupled options.

Member
josh replied Sep 7, 2011

Then you need to add another option for config.assets.caching.

Enabling sprockets production caching isn't the same thing as flipping digests off and on.

Member

Sounds good.

Member

Ancient history now, but 👍 to config.assets.caching to cache the environment (or the reverse, config.assets.reload ?)

Member

Right, the new option was never added and I think this is causing #6803

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment