CLI modularized, express injection allowed #576
Merged
Conversation
RubenVerborgh
approved these changes
Sep 5, 2017
Approved with minor change suggestions.
bin/lib/cli.js
Outdated
|
|
||
| module.exports = function cli (server) { | ||
| program | ||
| .version(packageJson.version) |
(nitpick) Make this one line?
bin/lib/cli.js
Outdated
| var loadInit = require('./init') | ||
| var loadStart = require('./start') | ||
|
|
||
| module.exports = function cli (server) { |
Could the function name be more descriptive? Because it doesn't return anything. Maybe startCli or something like this?
bin/lib/cli.js
Outdated
| var program = require('commander') | ||
| var packageJson = require('../../package.json') | ||
| var loadInit = require('./init') | ||
| var loadStart = require('./start') |
Perhaps all of these can become const now.
|
Thanks! |
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
As issued in #574
This is a simple modularization of CLI so it is possible to reuse it and inject own express like this:
EDIT: renamed cli() to startCli()
The text was updated successfully, but these errors were encountered: