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

Remove network request for pngquant src #80

Merged
merged 3 commits into from
Jul 19, 2018

Conversation

kevcenteno
Copy link
Contributor

@kevcenteno kevcenteno commented Apr 3, 2018

Problem

CI servers can sometimes fail to fetch the required bins and/or pngquant src.

Solution

  1. Put the pngquant src in the vendor directory
  2. Use binBuild.file instead of binBuild.src

@kevcenteno kevcenteno force-pushed the kevin/dont-fetch-bin branch 2 times, most recently from 867c507 to 85248f5 Compare April 3, 2018 01:51
@kevcenteno kevcenteno force-pushed the kevin/dont-fetch-bin branch 2 times, most recently from 127786d to 3ab1a7d Compare April 3, 2018 02:20
@kevcenteno kevcenteno changed the title Remove network requests for bins Remove network requests for pngquant src Apr 3, 2018
@kevcenteno kevcenteno changed the title Remove network requests for pngquant src Remove network request for pngquant src Apr 3, 2018
@kevcenteno
Copy link
Contributor Author

@sindresorhus Any thoughts?

@kevcenteno
Copy link
Contributor Author

@sindresorhus Is this still maintained?

@sindresorhus
Copy link
Contributor

You committed a change to vendor/linux/x64/pngquant. That needs to be removed.

@sindresorhus
Copy link
Contributor

I think this is a good idea 👍

@sindresorhus
Copy link
Contributor

You also need to add it to the files field in package.json

@kevcenteno kevcenteno force-pushed the kevin/dont-fetch-bin branch from 79972f3 to a0cd8c4 Compare April 30, 2018 16:12
@kevcenteno
Copy link
Contributor Author

@sindresorhus I've made the requested changes. Thank you for reviewing!

@chasebolt
Copy link

+1 on this as it would resolve a network issue i am having with this package. PR #83 can be closed out if this PR is merged in. this would also resolve #78.

package.json Outdated
@@ -33,7 +33,8 @@
"files": [
"cli.js",
"index.js",
"lib"
"lib",
"vendor/src-pngquant/pngquant-2.10.1-src.tar.gz"

Choose a reason for hiding this comment

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

Maybe just add vendor/src-pngquant? One less place to change when updating to a newer version.

@sindresorhus sindresorhus merged commit ae838c3 into imagemin:master Jul 19, 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