refactor(writing-styles): add postinstall script to download vale binary based on OS platform #131
Conversation
…ary based on platform
|
This pull request is being automatically deployed with ZEIT Now (learn more). 🔍 Inspect: https://zeit.co/commercetools/commercetools-docs-kit/ezhpju1rg |
| *.zip | ||
| *.tar.gz | ||
| bin/vale |
There was a problem hiding this comment.
We don't put it in version control anymore
| const destPath = path.join(__dirname, '../bin'); | ||
|
|
||
| console.log('[writing-styles] Verifying vale installation...'); | ||
| if (fs.existsSync(path.join(destPath, 'vale'))) { |
There was a problem hiding this comment.
We don't download it again if a binary file is already there.
|
@nkuehn alright please have another look, I re-wrote the script to be cross-platform compatible. |
emmenko
force-pushed
the
nm-writing-styles-postinstall-vale
branch
from
924649d
to
538ed22
Compare
emmenko
force-pushed
the
nm-writing-styles-postinstall-vale
branch
from
538ed22
to
5840ac1
Compare
| const path = require('path'); | ||
| const fetch = require('node-fetch'); | ||
| const shelljs = require('shelljs'); | ||
| const { path7za } = require('7zip-bin'); |
There was a problem hiding this comment.
Sorry to be stubborn., but this is a bit absurd - we're doing a refactor to get rid of 8-10MB sized binaries committed and to be able to do that we pull a package for unzipping them that is by itself a 9MB binary committed into NPM.
What was the original problem with committed binaries? If we think it's a bad pattern to work with committed binaries we shouldn't use 7zip-bin. If we deem the pattern OK why do we need this whole refactor?
I guess it's not to save CircleCI 20MB of network egress.
There was a problem hiding this comment.
we're doing a refactor to get rid of 8-10MB sized binaries committed and to be able to do that we pull a package for unzipping them that is by itself a 9MB binary committed into NPM
Yes it's a bit unfortunate but it's still better then before for the following reasons:
7zip-binis an external dependency, so we don't need to "deal" with the package size directly- our current package is
46.3MB as opposed to the7zip-binwhich is only8.84MB
- our package now is just a few kb, which makes publishing faster (and less error prone)
So I still think there is a good benefit of doing this.
There was a problem hiding this comment.
OK, it's not worth investing another hour to make it completely. It's odd-ish that uncompressing is so cumbersome in node, didn't expect that.
There was a problem hiding this comment.
The problem is not much about node, but uncompressing in windows
This PR further improves the
writing-stylespackage.I noticed that the
valebinary files takes up a few megabytes each, causing the package to be around 25Mb. This is a bit annoying when publishing and installing the package, as it just takes longer.We don't need to ship the binary with the package, instead we can use a
postinstallscript to download the binary "on demand" upon package installation, according to the OS platform. This also ensures that we only download the proper binary and not all of them.