Update for Rails 3.2 compatibility #1

Merged
merged 1 commit into from Mar 11, 2013

Conversation

Projects
None yet
4 participants
@boffbowsh
Contributor

boffbowsh commented Mar 8, 2013

.cache_page can take an optional gzip compression level, but we want to ignore this as we only ever use raw: true

@matsadler

This comment has been minimized.

Show comment Hide comment
@matsadler

matsadler Mar 11, 2013

looks good to me

looks good to me

lib/memcaches_page.rb
@@ -8,7 +8,8 @@ def cache_page(content, path, options={})
end
end
- def cache_page(content = nil, options = {})
+ def cache_page(content = nil, *args)
+ options = args.extract_options!

This comment has been minimized.

Show comment Hide comment
@mmazour

mmazour Mar 11, 2013

Is this right? My reading of the API is that extract_options! gives you what you want if options is the last argument, but for cache_page the last argument's the gzip level. (I think)

@mmazour

mmazour Mar 11, 2013

Is this right? My reading of the API is that extract_options! gives you what you want if options is the last argument, but for cache_page the last argument's the gzip level. (I think)

@boffbowsh

This comment has been minimized.

Show comment Hide comment
@boffbowsh

boffbowsh Mar 11, 2013

Contributor

Reading the Rails 3.2 API more closely it looks like they've broken the API completely for caches_page. I think I'll make this a proper breaking change and introduce memcaches_page instead so the intention is clear.

Contributor

boffbowsh commented Mar 11, 2013

Reading the Rails 3.2 API more closely it looks like they've broken the API completely for caches_page. I think I'll make this a proper breaking change and introduce memcaches_page instead so the intention is clear.

Paul Bowsher
Update for Rails 3.2 compatibility
Remove from the Rails `cache_page` pipeline 
entirely to avoid conflicts with the new gzip 
options in Rails 3.2
@boffbowsh

This comment has been minimized.

Show comment Hide comment
@boffbowsh

boffbowsh Mar 11, 2013

Contributor

Updated. Usable now with memcaches_page instead of caches_page. Tested with Member service locally.

Contributor

boffbowsh commented Mar 11, 2013

Updated. Usable now with memcaches_page instead of caches_page. Tested with Member service locally.

@mmazour

This comment has been minimized.

Show comment Hide comment
@mmazour

mmazour Mar 11, 2013

👍

mmazour commented Mar 11, 2013

👍

@timblair

This comment has been minimized.

Show comment Hide comment
@timblair

timblair Mar 11, 2013

Contributor

👍

Contributor

timblair commented Mar 11, 2013

👍

boffbowsh added a commit that referenced this pull request Mar 11, 2013

Merge pull request #1 from globaldev/rails-3.2
Update for Rails 3.2 compatibility

@boffbowsh boffbowsh merged commit 33c2d80 into master Mar 11, 2013

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