search

ericallam / font_assets

Helps with serving font assets with Rails 3.1
137 stars 79 forks source link

Change middleware insert order to work with config.threadsafe! #12

Closed nickurban closed 11 years ago

nickurban commented 11 years ago

I made this change in order to make font_assets work when running in threadsafe mode (which cannot insert before Rack::Lock because Rack::Lock is not in the middleware stack in threadsafe mode).

chrisnicola commented 11 years ago

Any reason why we don't just use config.middleware.use? Does it matter where this goes in the Rack stack. Seems like it could be brittle to specify which middleware it should go after.

nickurban commented 11 years ago

This was a while ago, but as I recall, the order did matter -- for instance with regard to Rack::Cache.

tfe commented 11 years ago

I ran into this problem too. Thanks @nickurban for the patch! I agree that the order matters.

tfe commented 11 years ago

For anyone happening upon this thread in the future, I have an updated fork of font_assets with @nickurban's patch here: https://github.com/tfe/font_assets.

nickurban commented 11 years ago

Happy to help :) And thank you for the updated version!

Perhaps this could be merged into the main branch?

ericallam commented 11 years ago

Thanks for updating this to work with threadsafe! We were trying to handle this before but I like this way better. New version is cut and on rubygems.org @ 0.1.9

tfe commented 11 years ago

See my note on the merge commit? Not sure if you meant to clobber the ActionDispatch::Static checks.

On Wed, Jul 3, 2013 at 4:46 PM, Eric Allam notifications@github.com wrote:

Thanks for updating this to work with threadsafe! We were trying to handle this before but I like this way better. New version is cut and on rubygems.org @ 0.1.9

— Reply to this email directly or view it on GitHubhttps://github.com/rubymaverick/font_assets/pull/12#issuecomment-20452022 .

ericallam commented 11 years ago

@tfe good catch, going to cut another one with a fix