-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Conversation
License: MIT Signed-off-by: Rasmus Erik Voel Jensen <github-rasmuserik@solsort.dk>
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.
Thank you for PR'ing this @rasmuserik ! 🙌🏽
will add too much boilerplate. Instead I just took the simplest approach, i.e. to instantiate an instance in the utils module, - and have a print function exported there.
Yes, makes sense.
If we want to have a quiet option, it could then be parsed once in bin.js, and then the logger might be instantiated from there.
I like that, could you move forward with it?
src/cli/commands/bootstrap/rm.js
Outdated
@@ -24,7 +26,7 @@ module.exports = { | |||
throw err | |||
} | |||
|
|||
list.Peers.forEach((l) => console.log(l)) | |||
list.Peers.forEach(peer => print(peer)) |
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.
small nitpick. Change to (peer) => print(peer)
. Lint should have caught that.
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.
Fixed in latest version of the pull request :)
src/cli/utils.js
Outdated
@@ -83,3 +83,4 @@ exports.createLogger = (visible) => { | |||
} | |||
} | |||
} | |||
exports.print = exports.createLogger(true) // TODO refactor/remove createLogger? |
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.
Yeah, go for it.
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.
done.
License: MIT Signed-off-by: Rasmus Erik Voel Jensen <github-rasmuserik@solsort.dk>
Yep, I will do that :) |
The It is implemented as a verbosity level, |
Mind adding tests for the new modes @rasmuserik ? We just need:
No need for multiple levels of 'verbosity' |
I'll add a test for the quiet flag, |
License: MIT Signed-off-by: Rasmus Erik Voel Jensen <github-rasmuserik@solsort.dk>
done :) |
Thank you @rasmuserik !! :D |
* refactor: CLI should use print instead of console.log #495 License: MIT Signed-off-by: Rasmus Erik Voel Jensen <github-rasmuserik@solsort.dk> * feat: add quiet/verbose CLI flag, - related to #931 License: MIT Signed-off-by: Rasmus Erik Voel Jensen <github-rasmuserik@solsort.dk> * add test, and remove verbosity levels License: MIT Signed-off-by: Rasmus Erik Voel Jensen <github-rasmuserik@solsort.dk>
Excluding `ipfs-http-client` in the `browser` field of package.json was causing Meteor to not include the bundle in it's browser build. fixes #10411 License: MIT Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
This changes the CLI to use a logger instead of console.out, as discussed in #495
There is a design decision about how we instantiate the logger: originally there was a logger instantiated in command/init, but I think that instantiating it in every command, will add too much boilerplate. Instead I just took the simplest approach, i.e. to instantiate an instance in the
utils
module, - and have a print function exported there.If we want to have a quiet option, it could then be parsed once in
bin.js
, and then the logger might be instantiated from there. Also if we intend always only have a single logger instance(?), we could removecreateLogger
, and just have the print function, plus the ability to configure whether to silence it or not.