optimus icon indicating copy to clipboard operation
optimus copied to clipboard

Refactor Logger

Open irainia opened this issue 2 years ago • 2 comments

Differently formatting message, such as coloring message, could help the user to notice on something important. But, the current implementation of providing log could be improved. As observed, the following is example of the current implementation of using log:

l.Info(coloredNotice("some message")) // with l is for log.Logger

the above example is taken from here

There's also another example like the following:

l.Warn(coloredNotice(fmt.Sprintf("some message %s", actualMessage))) // with l is for log.Logger

the above example is taken from here

The above example uses functions, as defined:

...
coloredNotice  = fmt.Sprintf
coloredError   = fmt.Sprintf
coloredSuccess = fmt.Sprintf
...

taken from here

With the current approach:

  • we need to handle formatting in separate fashion, like coloring in one function and formatting string message in another
  • although there might be a justifying reason, inconsistency in formatting (such as color) could rise (e.g. Info could be white or yellow or any other color)
  • in certain cases, we need to access the custom formatting function (like coloring function) if we want to use it outside of the cmd package (as currently majority of it is used here)

Our proposal is to address the above concerns. A more detailed approach could be finalized, but on the high level, log should handle any formatting (apart from string format value) behind the scene (not limited to coloring for standard output). For example in term of coloring:

logger.Info("some message from: %s", userName) // the message is written as plain white wherever it's called
logger.Warn("request message is halted by: %s", adminName) // the message is written as plain yellow wherever it's called
logger.Error("message expired as: %s", expirationReason) // the message is written as plain red wherever it's called

irainia avatar May 11 '22 01:05 irainia

@irainia salt has a printer module which provides these func. I would recommend us to use salt package for all cli outputs.

ravisuhag avatar May 11 '22 05:05 ravisuhag

@irainia salt has a printer module which provides these func. I would recommend us to use salt package for all cli outputs.

Sure, we can use any required libraries, including salt.

irainia avatar May 11 '22 06:05 irainia