romanbsd / heroku-deflater

Enable gzip compression on heroku, but don't compress images.
MIT License
358 stars 62 forks source link

Correct default max-age from 64,800 to 604,800 (18 hours -> 1 week) #27

Closed Capncavedan closed 7 years ago

Capncavedan commented 7 years ago

READY

It appears in this commit https://github.com/romanbsd/heroku-deflater/commit/c3f238eeed284573c45ccbc6ff1aef7b4671b0b4#diff-b3f6914ce0fa17bf552de724de2332ed that the default max-age was changed, I assume inadvertently, from a week (604,800) to 18 hours (64,800).

This PR corrects that, adds an underscore in the integer for clarity, and touches some tests to help avoid a regression (changes 64800 to something visually very different, 3_600)

romanbsd commented 7 years ago

It has always been 86400, which is 24h. And it's configurable. I prefer to keep it that way.

Capncavedan commented 7 years ago

@romanbsd if you wouldn't mind reconsidering - it looks like it was one week (604,800), and was changed to just 18 hours (64,800) in the commit I referenced.

From lib/heroku-deflater/railtie.rb in the diff of that commit:

screen_shot_2017-01-27_at_7_31_54_am
romanbsd commented 7 years ago

yep, you're right. I'll give it another thought and review the code paths again. In any case, it's configurable. I'll need to validate that.