-
Notifications
You must be signed in to change notification settings - Fork 82
Feature/gzip support #158
base: master
Are you sure you want to change the base?
Feature/gzip support #158
Conversation
ghost
commented
Jan 15, 2015
- Added location config for assets with gzip_static enabled Configure nginx to server static gzipped content #72
- Upgraded serverspec gem to most recent version
@@ -7,3 +7,11 @@ | |||
end | |||
|
|||
end | |||
|
|||
describe file('/etc/nginx/sites-available/intercity_sample_app.conf') do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
gzip_static on; | ||
expires max; | ||
add_header Cache-Control public; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpicking here, but the } is not placed correctly yet ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, I've got some issues with rendering partials, the first line is always shifted 2 spaces right
@oiuzikov Looks good! The only thing I'm missing is an option to enable/disable gzipped assets. So in the node JSON configuration we would want an option to enable it. |
@michiels what would be the default option? |
@oiuzikov i think disable it, because you need to do some rails configuring to make static gzip work so most people have it disabled by default |
@@ -53,7 +53,8 @@ | |||
"username": "<db username, max 10 characters>", | |||
"password": "<enter a random password>", | |||
"database": "<appname>_<stage>" | |||
} | |||
}, | |||
"gzip_enabled": true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't it be better to have the sample default to false
? Since people do have to perform some task on their rails apps to make this work
describe file("/etc/nginx/sites-available/intercity_sample_app.conf") do | ||
it { should be_file } | ||
|
||
its(:content) do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you but a context
block around this? so it tells a bit more about what you're testing. So you get something like:
describe file("/etc/nginx/sites-available/intercity_sample_app.conf") do
it { should be_file }
context "GZip support enabled" do
its(:content) do
should match /location ~ \^\/\(assets\)\/.*gzip_static on;/m
end
end
end