-
Notifications
You must be signed in to change notification settings - Fork 5
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
feat: make the brotli-size
dependency optional
#9
Conversation
b3e0082
to
02320a7
Compare
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.
Thanks for working on this! Some little things to be fixed...
lib/helpers/compressed-size.js
Outdated
brotli = require('brotli-size'); | ||
console.log(brotli); | ||
} catch (error) { | ||
console.log(error); |
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.
Please remove all console.log
output.
lib/helpers/compressed-size.js
Outdated
console.log(brotli); | ||
} catch (error) { | ||
console.log(error); | ||
throw new Error( |
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.
Please add this pkg to dependencies
and throw a SilentError
instead, which will have some better UX (no stack trace): https://github.com/ember-cli/silent-error
README.md
Outdated
@@ -15,6 +15,13 @@ ember install ember-cli-bundlesize | |||
|
|||
Make sure to `git add` the added `config/bundlesize.js` file! | |||
|
|||
#### Optional dependency | |||
|
|||
If you're app uses brotli compression and the `brotli` option is set in your configuration (i.e `compression: 'brotli'`), you must install the optional dependency: `brotli-size`. |
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.
If you're app uses brotli compression and the `brotli` option is set in your configuration (i.e `compression: 'brotli'`), you must install the optional dependency: `brotli-size`. | |
If your app uses Brotli compression and the `brotli` option is set in your configuration (i.e `compression: 'brotli'`), you must install the optional dependency: `brotli-size`. |
727c749
to
d0872f8
Compare
d0872f8
to
d24d80c
Compare
@simonihmig Addressed the feedback and build is now passing. Ready for another review! |
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.
Looks good! Thank you!
Published as 0.2.0! 🎉 |
Closes #8