-
Notifications
You must be signed in to change notification settings - Fork 3.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cmd/logcli: support load config from file #218
Conversation
Thanks for the PR @daixiang0! Looks like it need some more work, ping me when you've had a chance to address the feedback. |
067cbed
to
2891e28
Compare
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.
-
getConfig
should return with the results fromLoadConfig
- move
getConfig
call intomain
- handle errors and exit
9b0723d
to
0ade7a3
Compare
@davkal All done, please review again. |
Isn't the error logged twice now? Also, @gouthamve is this good code organization in the |
0ade7a3
to
31d65a9
Compare
Do you mean no need log in config.go? I have removed that.
This organization is what |
31d65a9
to
a2616a7
Compare
Thank you, we're almost there. Only the docs need cleaning up, then we can merge this. |
Done :) |
Signed-off-by: Xiang Dai <764524258@qq.com>
6d12ee6
to
2392cc3
Compare
Thank you for your contribution @daixiang0 🎉 |
Update from upstream repository incl. go1.20 compatibily patches
Support load config from file and move default config into here.
Signed-off-by: Xiang Dai 764524258@qq.com