Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add gzip task for production, fixes #712 #713

Merged
merged 1 commit into from
May 3, 2016
Merged

Conversation

fhemberger
Copy link
Contributor

@fhemberger fhemberger commented May 3, 2016

Ref: #712
/cc @jbergstroem

@jbergstroem
Copy link
Member

That was quick! LGTM (haven't tested but flags and formats looks good to me); here are the content types we (would) gzip, so it should match well: gzip_types text/plain text/css application/x-javascript text/xml application/xml application/xml+rss text/javascript;

@fhemberger
Copy link
Contributor Author

@jbergstroem You don't gzip text/html?

@jbergstroem
Copy link
Member

@fhemberger from nginx manual: "Responses with the “text/html” type are always compressed."

@fhemberger
Copy link
Contributor Author

@jbergstroem Ah, ok. Merging this now.

@fhemberger fhemberger merged commit e9f7544 into master May 3, 2016
@fhemberger fhemberger deleted the feature/gzip branch May 3, 2016 07:20
@jbergstroem
Copy link
Member

Cool. Does this mean that it'll be shipped with next version of docs/website?

@fhemberger
Copy link
Contributor Author

I guess all that's left to do is replacing the line here (https://github.com/nodejs/build/blob/master/setup/www/resources/scripts/build-site.sh#L28):

-  build_cmd="npm run build"
+  build_cmd="npm run build; npm run gzip"

@jbergstroem
Copy link
Member

Ok. Want to PR?

@fhemberger
Copy link
Contributor Author

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants