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

add networkId, windows support, ganache version, broken npm scripts #32

Merged
merged 1 commit into from
Dec 28, 2018

Conversation

dkent600
Copy link
Contributor

@dkent600 dkent600 commented Dec 22, 2018

Resolves: #30

  • hardcodes networkId to potentially help stabilize the ganache db, and to be consistent with what arc.js expects in order to identify our instance of ganache (Supply a constant "networkId" to ganache #30)
  • fixes broken npm script migrate
  • improves windows support for scripts (Windows support #3), not including the docker commands that still don't work in windows
  • fixes ganache error on migrate "Cannot read property 'pop' of undefined" by upgrading ganache-cli version

@dkent600 dkent600 self-assigned this Dec 22, 2018
@dkent600 dkent600 mentioned this pull request Dec 22, 2018
@dkent600 dkent600 requested review from orenyodfat and removed request for tsuberim December 22, 2018 12:58
Copy link
Contributor

@orenyodfat orenyodfat left a comment

Choose a reason for hiding this comment

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

please add tests

@dkent600
Copy link
Contributor Author

dkent600 commented Dec 23, 2018

@orenyodfat

please add tests

This repo has no travis tests, no test framework at all. There is already a ticket on that and it should be worked independently of this PR.

@orenyodfat
Copy link
Contributor

@orenyodfat

please add tests

This repo has no travis tests, no test framework at all. There is already a ticket on that and it should be worked independently of this PR.

https://github.com/daostack/migration/blob/master/.travis.yml
currently it is doing only linting.
please rebase your branch on top of master

@dkent600
Copy link
Contributor Author

@orenyodfat

please rebase your branch on top of master

done

@orenyodfat
Copy link
Contributor

@dkent600 which part of this pr support window ?

@ben-kaufman
Copy link
Contributor

@dkent600 please change back to the default --deterministic mnemonic as we discussed we would use it in all the stack repos.

@dkent600
Copy link
Contributor Author

@orenyodfat

which part of this pr support window ?

The use of rimraf and mkdirp in "scripts" in package.json.

@dkent600 dkent600 changed the title fix accounts, windows support, ganache version, broken npm scripts add networkId, windows support, ganache version, broken npm scripts Dec 26, 2018
merge fixes, lint

revert to deterministic

lint
@dkent600 dkent600 merged commit 0076778 into master Dec 28, 2018
@dkent600 dkent600 deleted the fixAccounts branch December 28, 2018 14:32
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.

Supply a constant "networkId" to ganache
3 participants