[major][feat] electrode-ignite modules #548
Conversation
| #!/usr/bin/env node | ||
|
|
||
| const Path = require("path"); | ||
| require(Path.join(__dirname, "../cli/ignite"))(); |
There was a problem hiding this comment.
no __dirname necessary. Just require("../cli/ignite") works.
| .then(function(latestVersion) { | ||
| var latestVersion = latestVersion.stdout.slice(0, -1); | ||
|
|
||
| if (latestVersion > pkg.version) { |
There was a problem hiding this comment.
can't compare semver like strings
| if (latestVersion > pkg.version) { | ||
| /* Case 1: electrode-ignite version outdated */ | ||
| logger.log( | ||
| chalk.yellow(`Latest version is ${latestVersion} - trying to update.`) |
There was a problem hiding this comment.
show current version and latest version
| logger.log( | ||
| chalk.yellow(`Latest version is ${latestVersion} - trying to update.`) | ||
| ); | ||
| xsh |
| } else { | ||
| /* Case 3: Invalid electrode-ignite version */ | ||
| errorHandler( | ||
| "Failed at: Invalid electrode-ignite version. Please verify your package.json." |
There was a problem hiding this comment.
this message is misleading. If there's issue with electrode-ignite version, it's our problem, has nothing to do with user's package.json and nothing they can verify.
First use proper semver comparison to check if version is newer, and then consider what error could occur and handle accordingly.
packages/ignite-core/ignite.js
Outdated
| } else { | ||
| errorHandler("Please provide a valid task name."); | ||
| } | ||
| } |
There was a problem hiding this comment.
Please use https://www.npmjs.com/package/yargs and provide proper command line options and usage helps.
| @@ -0,0 +1,42 @@ | |||
| "use strict"; | |||
There was a problem hiding this comment.
isn't this file also in electrode-ignite?
| }; | ||
|
|
||
| const Installation = function() { | ||
| checkXClapCLI().then(function(version) { |
| console.log( | ||
| `Electrode Ignite is about to install the following modules globally:\n- xclap-cli\n` | ||
| ); | ||
| installXClapCLI(); |
| ); | ||
| installXClapCLI(); | ||
| } else { | ||
| checkXClapCLILatestVersion().then(function(latestversion) { |
|
Many promises are not returned and left dangling. Please address comments and make sure all promises are chained properly. |
|
Please use |
…ack to main menu
| #!/usr/bin/env node | ||
|
|
||
| const Path = require("path"); | ||
| require(Path.join("../cli/ignite"))(); |
packages/ignite-core/ignite.js
Outdated
| return taskLoader("1", type, igniteCore); | ||
| } else if (task === "check-nodejs") { | ||
| return taskLoader("2", type, igniteCore); | ||
| } else if (task === "generate-app") { |
There was a problem hiding this comment.
should find a way to avoid a long chain of if/else constructs that's all similar.
packages/ignite-core/lib/menu.js
Outdated
| } | ||
| } | ||
| ret += chalk.blueBright("Electrode Ignite Menu"); | ||
| for (let i = 0; i < STARNUM; i++) { |
There was a problem hiding this comment.
this block of code looks exactly like it's above line 23-29
packages/ignite-core/lib/menu.js
Outdated
| `[7] \u261E Exit` | ||
| ]; | ||
|
|
||
| console.log(chalk.blueBright(dashedLines)); // eslint-disable-line no-console |
There was a problem hiding this comment.
I'd just /* eslint-disable no-console */ at top of the file.
packages/ignite-core/lib/menu.js
Outdated
| option = answer; | ||
|
|
||
| // Invalid Electrode Option will re-trigger the menu | ||
| while (option < 1 || option > options.length || isNaN(option)) { |
| let yoPath = ""; | ||
| let child = ""; | ||
|
|
||
| if(process.platform.startsWith("win")) { |
| return xsh | ||
| .exec(true, "npm ls -g -j --depth=0 xclap-cli") | ||
| .then(function(ret) { | ||
| resolve(JSON.parse(ret.stdout).dependencies["xclap-cli"].version); |
There was a problem hiding this comment.
xsh.exec returns a promise. You don't need to create another promise like this.
| return xsh | ||
| .exec(true, "npm show xclap-cli version") | ||
| .then(function(version) { | ||
| resolve(version.stdout.slice(0, -1)); |
There was a problem hiding this comment.
xsh.exec returns a promise. You don't need to create another promise like this.
| return igniteCore(type); | ||
| } | ||
| return ; | ||
| } else if (semverComp(version, latestversion) < 0) { |
There was a problem hiding this comment.
you are calling semverComp repetitively for the same input.
| return igniteOutdated(latestVersion); | ||
|
|
||
| /* Case 2: electrode-ignite latest version */ | ||
| } else if (semverComp(latestVersion, pkg.version) === 0) { |
There was a problem hiding this comment.
you are calling semverComp repetitively for the same input.
A bootstrap tool for installing, updating, and assiting development with OSS Electrode platform.
PR is along with the gitbook: #552