-
-
Notifications
You must be signed in to change notification settings - Fork 359
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
feat(progamableApi): CMD options can be passed programatically now #555
Conversation
Signed-off-by: Marc Bornträger <marc.borntraeger@gmail.com>
@wzrdtales The new PR with your suggested changes. Could you maybe release a new version if you think it's good? 😄 |
@wzrdtales and do you know why the CI fails? 😅 |
lib/commands/set-default-argv.js
Outdated
@@ -98,9 +98,10 @@ module.exports = function (internals, isModule) { | |||
.describe('log-level', 'Set the log-level, for example sql|warn') | |||
.string('log-level'); | |||
} else { | |||
var _internalsArgv = Object.assign(defaultConfig, internals.cmdOptions); |
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.
we stop using var from now on, since we dropped support prior node 4. Use const here instead.
Arggh... Seems the node team backported some features to node 8
|
I pushed out a change that fixes the ci errors. You will need to rebase against it. |
Signed-off-by: BorntraegerMarc <marc.borntraeger@gmail.com>
@wzrdtales didn't know that node v4 support was dropped. But I think it's very cool :) Ready for anoter review |
everything before node 4, node 4 itself is supported though |
Looks good, please bare with me, I have some meetings now, but will publish ASAP. Ping me should I not publish it before tomorrow morning :) |
Pushed out. For the next time few hints: Here you can find a description of the conventions for commits. Your second commit would have better suited the refactor type :) |
|
@wzrdtales noted. thanks for the tip :) |
Fixes #553