-
-
Notifications
You must be signed in to change notification settings - Fork 159
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add gzip for assets as well as server responses #983
Conversation
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.
This is so simple. I love it 👍 Just a couple suggestions
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.
A few small suggestions. Overall this is awesome. Can't wait to get this in people's hands! Thank you
Add explicit return type for should_compress? Co-Authored-By: Paul Smith <[email protected]>
@paulcsmith, all the issues have been resolved. I appreciate your patience as I learn crystal and lucky! |
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.
Just one tiny comment then LGTM! Nice work on this
There we go... |
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.
Perfect! 🎉
* added static asset compression to lucky apps * settings for dynamic and static gzipping combined luckyframework#983 * added wrong branch name in dependency * only compress static assets in production * add new line to end of package.json * Update src/browser_app_skeleton/webpack.mix.js Capitalization fix Co-Authored-By: Paul Smith <[email protected]> * Update src/browser_app_skeleton/webpack.mix.js Capitalization fix Co-Authored-By: Paul Smith <[email protected]> * Update src/web_app_skeleton/config/server.cr.ecr Capitalization fix Co-Authored-By: Paul Smith <[email protected]> * Update src/web_app_skeleton/config/server.cr.ecr Capitalization fix Co-Authored-By: Paul Smith <[email protected]> * fixed some whitespace issues Co-authored-by: Paul Smith <[email protected]>
Purpose
Gzip Lucky::TextResponse if it's enabled #154
Description
@paulcsmith Lucky::TextResponse was definitely the right place to do this. The implementation was trivial. Once the static compression PR and this PR are merged, I will add a config file to the lucky_cli repo that enables this in new apps and contains comments that explain how it works.
Checklist
crystal tool format spec src
./script/setup
./script/test