[Tooling] CLI: make sure outputs to the user are not `log` package calls

deblasis commented 2 years ago

Objective

Make sure that the CLI outputs to the user in the best way possible.

Currently, we are not logging anything inside the CLI, we might in the future, at that point we'll have to distinguish between that's UX and logging but that's another story for another day. KISS for now I'd say.

Origin Document

A discussion around the use of log vs fmt package functions when printing stuff to StdOut/StdErr

Explanation of the problem here

Tiny action Item here. Creating the issue just to log this and to finally push #177 out without additional scope creep.

Goals

[ ] Replace log package calls to fmt where appropriate in the context of the CLI.

Deliverable

[ ] Updated CLI with direct output to StdOut/StdErr and no log

Non-goals / Non-deliverables

...

General issue deliverables

[ ] Update the appropriate CHANGELOG
[ ] Update any relevant READMEs (local and/or global)
[ ] Update any relevant global documentation & references
[ ] If applicable, update the source code tree explanation
[ ] If applicable, add or update a state, sequence or flowchart diagram using mermaid

[Optional] Testing Methodology

Task specific tests: make ...
All tests: make test_all
LocalNet: verify a LocalNet is still functioning correctly by following the instructions at docs/development/README.md

Creator: @deblasis Co-Owners: ??

Olshansk commented 2 years ago

@deblasis Tickets like this are fair to just throw into the "Quality of Life" milestone in the V1 Dashboard

Olshansk commented 1 year ago

@h5law Do you have any opinions here? I know you've looked into fmt vs log in the CLI before

h5law commented 1 year ago

Do you have any opinions here? I know you've looked into fmt vs log in the CLI before

@Olshansk so I was actually thinking about this recently. I used logger.Global for output in some of the CLI for the keybase and am starting to think this should be changed to fmt.Printf statements as they are far more readable.

We have this line (57) in runtime/configs/config.go which really frustrates me

log.Default().Printf("No config provided, using defaults")

Basically whenever we query the RPC for lets say the current supply this will be printed before out JSON is returned.

I think a good starter task would be to go through the CLI and change any logs that represent information the user is trying to read (like p1 Keys List for example) and change the outputs to fmt.Println statements. I think anything that is not directly feeding the user information that they would like to see should instead be logged to os.Stderr this way we can redirect any unwanted logs as follows p1 Keys List 2>/dev/null this will show all sucessfull output (stdout) and hide any logs/errors (stderr).

Not entirely set on this so open to a change of mind, but I really hate that config line ngl

Olshansk commented 1 year ago

Do you have any opinions here? I know you've looked into fmt vs log in the CLI before

@Olshansk so I was actually thinking about this recently. I used logger.Global for output in some of the CLI for the keybase and am starting to think this should be changed to fmt.Printf statements as they are far more readable.

We have this line (57) in runtime/configs/config.go which really frustrates me
log.Default().Printf("No config provided, using defaults")
Basically whenever we query the RPC for lets say the current supply this will be printed before out JSON is returned.

I think a good starter task would be to go through the CLI and change any logs that represent information the user is trying to read (like p1 Keys List for example) and change the outputs to fmt.Println statements. I think anything that is not directly feeding the user information that they would like to see should instead be logged to os.Stderr this way we can redirect any unwanted logs as follows p1 Keys List 2>/dev/null this will show all sucessfull output (stdout) and hide any logs/errors (stderr).

Not entirely set on this so open to a change of mind, but I really hate that config line ngl

On the same page, you down to take this on?

h5law commented 1 year ago

@Olshansk Sure thing will get the PR up tmrw <3

pokt-network / pocket