-
Notifications
You must be signed in to change notification settings - Fork 38
WIP: Relates to #81. Update to Electron v4.1.0 #91
Conversation
…ctron 4 * Add contributor instructions for forking, cloning, testing, linting, and building * Add contributor instructions for checking if dependencies need updating * Add contributor instructions for updating dependencies and checking that latest library updates integrate with other repos by adding a version number to the package.json so that it may be installed from a remote branch. * Add minimum Node.js version compatible with Electron 4.
…tsc when install as dependency
…inary error when install as dependency
* Update to Electron v3.1.0 - https://github.com/electron/electron/releases * Update Readme * Update dependencies versions * Add .nvmrc
Otherwise when try to use the package from a remote branch such as ``` "@parity/electron": "https://github.com/paritytech/js-libs.git#luke-81-electron-3", ``` Then it produces error: ``` error Can't add "@parity/electron": invalid package version undefined. ```
When I add this to the fether-electron package.json dependency with:
```
"@parity/electron": "https://github.com/paritytech/js-libs.git#luke-81-electron-3",
```
And then try to run `yarn; yarn build;`, it gives the following error:
```
scon @ ~/code/src/paritytech/fether - [luke-309-electron-3] $ yarn; yarn build
yarn install v1.12.3
[1/5] 🔍 Validating package.json...
[2/5] 🔍 Resolving packages...
[3/5] 🚚 Fetching packages...
[4/5] 🔗 Linking dependencies...
warning " > [email protected]" has unmet peer dependency "eslint@>= 4.12.1".
warning " > [email protected]" has unmet peer dependency "prop-types@^15.6.1".
warning " > [email protected]" has unmet peer dependency "react@^16.4.0".
warning "workspace-aggregator-cc67e372-3487-4efc-8fcb-a403fb15962c > fether-react > @parity/[email protected]" has incorrect peer dependency "rxjs@~6.2.2".
warning "workspace-aggregator-cc67e372-3487-4efc-8fcb-a403fb15962c > fether-react > @parity/[email protected]" has incorrect peer dependency "rxjs@~6.2.2".
warning "workspace-aggregator-cc67e372-3487-4efc-8fcb-a403fb15962c > fether-react > [email protected]" has unmet peer dependency "prop-types@^15.6.0".
warning "workspace-aggregator-cc67e372-3487-4efc-8fcb-a403fb15962c > fether-react > [email protected]" has unmet peer dependency "prop-types@^15.6.0".
warning "workspace-aggregator-cc67e372-3487-4efc-8fcb-a403fb15962c > fether-react > @babel/[email protected]" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "workspace-aggregator-cc67e372-3487-4efc-8fcb-a403fb15962c > fether-ui > [email protected]" has unmet peer dependency "react-dom@>=0.14.0 <= 16".
warning "workspace-aggregator-cc67e372-3487-4efc-8fcb-a403fb15962c > fether-react > @babel/plugin-proposal-decorators > @babel/[email protected]" has unmet peer dependency "@babel/core@^7.0.0-0".
[5/5] 📃 Building fresh packages...
[1/2] ⠄ @parity/electron
error /Users/scon/code/src/paritytech/fether/node_modules/@parity/electron: Command failed.
Exit code: 1
Command: yarn build
Arguments:
Directory: /Users/scon/code/src/paritytech/fether/node_modules/@parity/electron
Output:
yarn run v1.12.3
$ lerna exec yarn build --stream
lerna notice cli v3.10.1
lerna info Executing command in 6 packages: "yarn build"
@parity/abi: $ rimraf lib
@parity/electron: $ rimraf lib
@parity/abi: $ tsc
@parity/abi: /bin/sh: tsc: command not found
@parity/electron: $ tsc
@parity/abi: error Command failed with exit code 127.
@parity/abi: info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
@parity/electron: /bin/sh: tsc: command not found
lerna ERR! yarn build exited 1 in '@parity/abi'
lerna WARN complete Waiting for 1 child process to exit. CTRL-C to exit immediately.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
```
|
@ltfschoen Can I close #90? |
Yes I've closed it now |
|
@amaurymartiny I wonder why the CI is failing with |
|
@amaurymartiny I don't think I can progress this PR without first resolving #58. |
amaury1093
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, except for the CI error.
I can manually update packages to npm, I didn't do it until now because the electron package didn't change. But I will bump the version on npm once this is merged.
So #58 is not a blocker, just a small (and quite weird) annoyance.
packages/electron/package.json
Outdated
| "@types/async-retry": "^1.2.1", | ||
| "@types/checksum": "^0.1.30", | ||
| "electron": "^2.0.2" | ||
| "electron": "^2.0.2 | ^3.1.0 | ^4.0.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
^4.0.1 here.
Solves:
I wonder why the CI is failing with error Couldn't find any versions for "electron" that matches "^2.0.2 | ^3.1.0 | ^4.0.1"
…n version Tries to solve the following error when push to Github and CI runs ``` CI is failing with error Couldn't find any versions for "electron" that matches "^2.0.2 | ^3.1.0 | ^4.0.1" ```
|
@amaurymartiny It appears to be passing CI now since applying the changes to suggested, except for coveralls |
package.json
Outdated
| { | ||
| "name": "js-libs", | ||
| "description": "A collection of JavaScript libraries for dapp development.", | ||
| "version": "v3.0.26", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this.
I just tried a lerna version patch --push false locally, it bumps all packages correctly, but doesn't bump this. I'm pretty sure lerna doesn't need a version here.
|
No problem for coveralls |
packages/contracts/package.json
Outdated
| "docs": "typedoc && rimraf docs/README.md", | ||
| "prebuild": "rimraf lib", | ||
| "build": "tsc", | ||
| "build": "yarn && ./node_modules/.bin/tsc", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the late comment, but i just (re-)saw this. With the new step 7 in the readme, tsc command is enough.
amaury1093
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Luke!
Incorporates review comments from the PR for Electron v3.1.0 WIP: Relates to #81. Update to Electron v3.1.0 #90
fether terminal tab
"@parity/electron": "^3.0.1",)match the version number in each js-lib package's package.json version number