review-fix: Updates based on review comments so far

openethereum · ltfschoen · Jan 9, 2019 · Jan 9, 2019 · Jan 9, 2019 · Jan 10, 2019
commit 2e2d8090454ef7b8731b386c1e56f77040fb827f
diff --git a/.nvmrc b/.nvmrc
@@ -1 +1 @@
-10.11.0
+10.10.0
diff --git a/README.md b/README.md
@@ -35,7 +35,7 @@ And below are the lower-level packages, used internally, or by advanced users.
 
 #### Dependencies
 
-Install at least `yarn` version 1.4.2 and [Node.js >=10.11.0](https://nodejs.org/en/)
+Install at least `yarn` version 1.4.2 and [Node.js >=10.10.0](https://nodejs.org/en/)
 
 ```
 yarn --version // Should be at least 1.4.2
@@ -44,13 +44,13 @@ yarn --version // Should be at least 1.4.2
 #### Tests
 
 ```
-export NPM_TOKEN=''; yarn test
+yarn test
 ```
 
 #### Build
 
 ```
-export NPM_TOKEN=''; yarn build
+yarn build
 ```
 
 #### Maintenance
@@ -65,10 +65,10 @@ git clone https://github.com/<INSERT_YOUR_GITHUB_USERNAME>/js-libs
 3. Check outdated dependencies
 
 ```
-npm outdated
+yarn outdated
 ```
 
-4. Create a branch and update any dependencies that it indicates are out of date.
+4. Create a branch
 
 ```
 git checkout -b <INSERT_YOUR_BRANCH_NAME>
@@ -77,7 +77,7 @@ git checkout -b <INSERT_YOUR_BRANCH_NAME>
 5. Run tests, linting, and build
 
 ```
-export NPM_TOKEN=''; yarn test; yarn lint; yarn build
+yarn test; yarn lint; yarn build
 ```
 
 6. Push the branch to your fork of the repo

diff --git a/package.json b/package.json
@@ -19,7 +19,7 @@
     ]
   },
   "engines": {
-    "node": ">=10.11.0",
+    "node": ">=10.10.0",
     "yarn": "^1.4.2"
   },
   "scripts": {

diff --git a/packages/electron/package.json b/packages/electron/package.json
@@ -33,10 +33,10 @@
   "devDependencies": {
     "@types/async-retry": "^1.2.1",
     "@types/checksum": "^0.1.30",
-    "electron": "^3.1.0",
+    "electron": "^2.0.2 | ^3.1.0 | ^4.0.1",
     "typescript": "^3.1.6"
   },
   "peerDependencies": {
-    "electron": "^3.1.0"
+    "electron": "^2.0.2 | ^3.1.0 | ^4.0.1"
   }
 }
Original file line number	Diff line number	Diff line change
		@@ -1 +1 @@
		10.11.0
		10.10.0
Copy link Collaborator amaury1093 Jan 11, 2019 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. My questions are: If your nvm has v11, does it change to v10.10.0? (not necessary) Is it enough to only put `"engine"` in package.json? ltfschoen reacted with thumbs up emoji Copy link Contributor Author ltfschoen Jan 13, 2019 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. You're right. I'll remove the .nvmrc file. It's more useless than I thought If I'm using the wrong Node.js version it warns me when I run `yarn` as follows: `scon @ ~/code/src/paritytech/js-libs - [luke-81-electron-4] $ nvm use 10.6.0 Now using node v10.6.0 (npm v6.1.0) scon @ ~/code/src/paritytech/js-libs - [luke-81-electron-4] $ yarn yarn install v1.12.3 [1/5] 🔍 Validating package.json... error [email protected]: The engine "node" is incompatible with this module. Expected version ">=10.10.0". Got "10.6.0" error Found incompatible module info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.` You're right, there's just as many steps whether we do or don't have the .nvmrc file. The important thing is to have the "engine" specified. `scon @ ~/code/src/paritytech/js-libs - [luke-81-electron-4] $ node --version v10.6.0 scon @ ~/code/src/paritytech/js-libs - [luke-81-electron-4] $ nvm use No .nvmrc file found` With .nvmrc file: `scon @ ~/code/src/paritytech/js-libs - [luke-81-electron-4] $ node --version v10.6.0 scon @ ~/code/src/paritytech/js-libs - [luke-81-electron-4] $ nvm use Found '/Users/scon/code/src/paritytech/js-libs/.nvmrc' with version <10.10.0> N/A: version "10.10.0 -> N/A" is not yet installed. You need to run "nvm install 10.10.0" to install it before using it. scon @ ~/code/src/paritytech/js-libs - [luke-81-electron-4] $`