Skip to content
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

fix: pass docusaurus-publish cli args to build command #1185

Merged
merged 2 commits into from
Jan 15, 2019

Conversation

zkochan
Copy link
Contributor

@zkochan zkochan commented Jan 14, 2019

The --skip-image-compression flag is passed thru to the build command

Motivation

(Write your motivation here.)

Have you read the Contributing Guidelines on pull requests?

(Write your answer here.)

Test Plan

(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Bonus points for screenshots and videos!)

Related PRs

(If this PR adds or changes functionality, please take some time to update the docs at https://github.com/facebook/docusaurus, and link to your PR here.)

The --skip-image-compression flag is passed thru to the build command
@docusaurus-bot
Copy link
Contributor

docusaurus-bot commented Jan 14, 2019

Deploy preview for docusaurus-preview ready!

Built with commit 5faff84

https://deploy-preview-1185--docusaurus-preview.netlify.com

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Jan 14, 2019
Copy link
Contributor

@JoelMarcey JoelMarcey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@endiliey @yangshun

Does this mean that --skip-image-compression never worked? Maybe because everyone was just ok having their images compressed 🤣 .

This looks good to me, but wanted to see what either of you thought.

@endiliey endiliey changed the title fix: --skip-image-compression fix: pass cli args to docusaurus-publish Jan 15, 2019
@endiliey endiliey changed the title fix: pass cli args to docusaurus-publish fix: pass docusaurus-build cli args to docusaurus-publish Jan 15, 2019
@endiliey endiliey changed the title fix: pass docusaurus-build cli args to docusaurus-publish fix: pass docusaurus-publish cli args to build command Jan 15, 2019
Copy link
Contributor

@endiliey endiliey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JoelMarcey

Does this mean that --skip-image-compression never worked? Maybe because everyone was just ok having their images compressed 🤣 .

It never worked for docusaurus-publish. But it always worked for docusaurus-build.

LGTM.

Thanks for doing this 😄

@yangshun
Copy link
Contributor

I think we might not have tested with the publish command, only with the build command in #654.

@endiliey
Copy link
Contributor

endiliey commented Jan 15, 2019

I did a small test. This should be OK to merge

Adding short debug line at generate.js

console.log(commander.skipImageCompression ? 'skipped': 'not skipped');
process.exit(1);

No flag
image

With flag
image

@endiliey endiliey merged commit e31ecdf into facebook:master Jan 15, 2019
@zkochan zkochan deleted the patch-3 branch January 15, 2019 10:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants