Logger framework and trade.go example #88
Conversation
|
I left the logger variable as a top-level declaration in trade.go because if we move it into |
cmd/trade.go
Outdated
| log.Println() | ||
| log.Printf("problem encountered while instantiating the fill tracker: %s\n", e) | ||
| l.Info("") | ||
| l.Infof("problem encountered while instantiating the fill tracker: %s\n", e) |
There was a problem hiding this comment.
this should follow the same pattern as the logging of the monitoring server to keep things consistent. either change both to a new pattern or none.
|
Changes made |
cmd/trade.go
Outdated
| } | ||
|
|
||
| if *logPrefix != "" { | ||
| t := time.Now().Format("20060102T150405MST") | ||
| fileName := fmt.Sprintf("%s_%s_%s_%s_%s_%s.log", *logPrefix, botConfig.AssetCodeA, botConfig.IssuerA, botConfig.AssetCodeB, botConfig.IssuerB, t) | ||
| e = setLogFile(fileName) | ||
| if e != nil { | ||
| log.Println() | ||
| log.Fatal(e) | ||
| l.Info("") |
There was a problem hiding this comment.
can remove the empty line printout here -- logger.Fatal starts off by printing an empty line
|
Updated |
| log.Printf("unable to start the monitoring server or problem encountered while running server: %s\n", e) | ||
| l.Info("") | ||
| l.Info("unable to start the monitoring server or problem encountered while running server:") | ||
| l.Errorf("%s", e) |
There was a problem hiding this comment.
this should typically be in the same line as where we wrap it (i.e. the line above), and should have a newline after (like we do on the left hand side of this diff). ok for now.
| // Fatal is a convenience method that can be used with any Logger to log a fatal error | ||
| func Fatal(l Logger, e error) { | ||
| l.Info("") | ||
| l.Errorf("%s", e) |
There was a problem hiding this comment.
this should typically include a newline character at the end l.Errorf("%s", e) -> l.Errorf("%s\n", e)
|
@Reidmcc this is really good -- feel free to move forward and convert the remaining files to this new pattern. |
|
this PR starts the process of addressing #9 |
This is a replacement for #71, incorporating requested changes.