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

Bump bin-wrapper to version 4 #89

Closed
wants to merge 1 commit into from
Closed

Bump bin-wrapper to version 4 #89

wants to merge 1 commit into from

Conversation

realityking
Copy link

@realityking realityking commented Jul 23, 2018

This should be good as a minor release.

@patsplat
Copy link

It doesn't work as a minor release pngquant-bin returns a BinWrapper, and bin-wrapper 4.x changes the public api.

That's at least part of why the post-installs script is failing.

@realityking
Copy link
Author

Test fails because of this bug: kevva/bin-wrapper#69

While bin-wrapper's public API did change, this module only returns the path. Thus no breaking change.

pngquant-bin/index.js

Lines 1 to 2 in 30d0f40

'use strict';
module.exports = require('./lib').path();

@LOK-Soft
Copy link
Contributor

any progress here, or do we need to wait for kevva/bin-wrapper#69 ?
As I understand there is a test failing, which checks the api, but there is no need to have the unchanged api for our purpose... did I get this right?

@LOK-Soft
Copy link
Contributor

LOK-Soft commented Oct 30, 2018

kevva/bin-wrapper#69 was merged in version 4.0.1

@1000ch
Copy link
Contributor

1000ch commented Oct 30, 2018

@realityking
bin-wrapper now use Promise interface, so run() returns Promise. Please also fix install.js like imagemin/gifsicle-bin#100.

@LOK-Soft
Copy link
Contributor

LOK-Soft commented Oct 30, 2018

@realityking
bin-wrapper now use Promise interface, so run() returns Promise. Please also fix install.js like imagemin/gifsicle-bin#100.

I just opened a new PR #95 with new bin-wrapper version 4.0.1 and Change to promise.

@1000ch
Copy link
Contributor

1000ch commented Oct 31, 2018

Fixed on #95, sorry 😢

@1000ch 1000ch closed this Oct 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants