-
Notifications
You must be signed in to change notification settings - Fork 66
log: fix log file path #345
Conversation
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.
LGTM
lightning/config/global.go
Outdated
@@ -197,6 +197,9 @@ func LoadGlobalConfig(args []string, extraFlags func(*flag.FlagSet)) (*GlobalCon | |||
if *logFilePath != "" { | |||
cfg.App.Config.File = *logFilePath | |||
} | |||
if cfg.App.Config.File == "" { |
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.
if cfg.App.Config.File == "" { | |
else { |
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.
This else is not ok, because toml config may set the log config path
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.
there should still be a way to explicitly log to terminal
Then I think we should leave the default to stdout |
We could make |
So shall we need to update docs for this special log config. Seems br special env |
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.
LGTM, @lichunzhu PTAL again.
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.
LGTM
What problem does this PR solve?
fix lightning always set log file path to the default path
What is changed and how it works?
Only set log path to default path if neither command line params nor config file set it
Check List
Tests
Side effects
Related changes