-
-
Notifications
You must be signed in to change notification settings - Fork 108
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: remove new redundant options arg from algorithm #26
Conversation
Remove unneeded module.exports
Permit {path} and {query} in asset name
Add zopfli option
use [] instead of {} rename `regExp` to `test`
Update index.js
…_zopfli Update node-zopfli version
- Previous Copyright holder is Tobias, this was transferred with the initial group to JSF
…-JSFMaintainers docs(readme): updates for JSF maintainers
…-JSFLicense chore(license): update to JSF standard license
@h4l In any case please close and reopen the PR to trigger the CLA Bot again 😛 |
I would go with this over #27 . You have to communicate the change well so people how to migrate and it has to be released as a major version. |
@h4l friendly ping :) |
@michael-ciniawsky sorry for the delay, I've signed the CLA now. Would you like me to rebase up to the current master? |
@h4l yep, please rebase 😛 |
It was introduced in 0.3.1 and broke configurations passing an explicit algorithm function, as the signature used to be function(buff, cb).
e65fd0d
to
5fd83a7
Compare
@michael-ciniawsky I've rebased. Seems like we could do with some tests though. |
@h4l There are no tests for |
I'm starting to feel like a broken record with this but if we are going to cut a major, we will need to do it along with defaults. |
Close due inactive. Feel free recreate new PR. Thanks! |
It was introduced in 0.3.1 and broke configurations passing an explicit
algorithm function, as the signature used to be
function(buff, cb)
.This implements option 1 from #25.