This repository has been archived by the owner on Feb 26, 2024. It is now read-only.
Move and optimize build process from ganache-cli to ganache-core
#162
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…global _scratch var
Also changing the build process to require that tests are run and pass before a publish.
…catch with fallback
… strict version checks on our optionalDeps.
mikeseese
approved these changes
Sep 10, 2018
There was a problem hiding this comment.
Reviewed this with @davidmurdoch over Zoom. LGTM! Would merge again 10/10
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request copies the build process from
ganache-clitoganache-corewhile also optimizing the resulting webpacked bundle to only includeganache-core's internals plus pure-js implementations of our native transitive dependencies. All other dependencies are installed oninstall.Additionally, ganache-core and ganache-cli will now default to using native dependencies, if successfully
installed, only falling back to the pure-js implementations if those dependencies can't be installed (i.e., because node-gyp on Windows is tricky).This PR also purges some unnecessary
dependenciesfrom package.json, moves others todevDependencies, and updates and pins the remaining direct dependencies to their latest patch versions (at time of writing), with the exception of:level-sublevel; as version6.6.5needlessly regressed to a dependency on a vulnerable version oflevelup.6.6.5actually looks like it was accidentally published from an older branch namedmaster2.¯\_(ツ)_/¯ethereumjs-tx; as version1.3.5introduced bug Self signed TXs doesn't work ethereumjs/ethereumjs-tx#113 which was not fixed in1.3.7in my testing.On
npm publishour tests will automatically be run, the webpack build created, and the tests run again on the new webpacked build. We may want to just trust that Travis will build and test everything for us, but having everything run onpublishensures that we don't ship an old or broken./builddirectory (since./buildwill not be created onnpm install).Our new
.npmignorewill ignore most of our unneeded files and directories on npm publish, but it will include./liband./build.Before
npm publishing to production we'll want to test the interaction of the new build steps, our.npmignore, and./buildon a variety of OSes (mainly Windows in different configurations to ensure the non-native fallback method works appropriately), perhaps by temporarily publishing to npm under a different name.With all that said and done, once axic/scrypt.js#3 is merged and published to npm we'll likely get rid of our node webpack build all together; currently, the only other consideration is
eth-block-tracker, which is potentially node 6 incompatible, so we may still want to use its es5 build. Note:secp256k1already automatically falls back to a pure-JS implementation if a native build isn't possible.