Skip to content

feat(log): add exra cmd-flags for file logger #56

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 20 commits into from
Aug 5, 2023
Merged

feat(log): add exra cmd-flags for file logger #56

merged 20 commits into from
Aug 5, 2023

Conversation

piyoki
Copy link
Contributor

@piyoki piyoki commented Aug 2, 2023 •
edited
Loading

Background

Add extra cmd flags to allow end-users to customize settings for file logger.

Ref: https://github.com/natefinch/lumberjack

Checklist

Full changelogs

  • feat(pkg/log): add options for fileLogger
  • feat(common/consts): persist default vals for log pkg
  • feat(cmd/{server,client}): add extra flags for fileLogger
  • docs(server,client): add extra logger auguments

Issue reference

NA

Test Result

Run without logFileWriter:

image

Run with logFileWriter:

image

@piyoki piyoki requested a review from a team as a code owner August 2, 2023 04:57
@mzz2017
Copy link
Contributor

mzz2017 commented Aug 2, 2023

Does it really work for gliderLog?

@piyoki
Copy link
Contributor Author

piyoki commented Aug 2, 2023

Does it really work for gliderLog?

It ONLY works for fileLogWriter. More precisely, if --log-enable-file-writer is NOT passed, the app will ONLY use ConsoleWriter.

@juicity-bot
Copy link
Contributor

juicity-bot bot commented Aug 3, 2023

❌ Your branch is currently out-of-sync to main. No worry, I will fix it for you.

@juicity-bot
Copy link
Contributor

juicity-bot bot commented Aug 5, 2023

❌ Your branch is currently out-of-sync to main. No worry, I will fix it for you.

mzz2017
mzz2017 reviewed Aug 5, 2023
View reviewed changes
@piyoki piyoki requested a review from mzz2017 August 5, 2023 13:27
@piyoki
Copy link
Contributor Author

piyoki commented Aug 5, 2023

LGTM, ready to merge?

@mzz2017
Copy link
Contributor

mzz2017 commented Aug 5, 2023 •
edited
Loading

Test Result:

image

mzz2017
mzz2017 previously approved these changes Aug 5, 2023
View reviewed changes
@mzz2017 mzz2017 merged commit f29909b into main Aug 5, 2023
@mzz2017 mzz2017 deleted the file_log_options branch August 5, 2023 14:39
@juicity-svc-infra juicity-svc-infra mentioned this pull request Aug 5, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mzz2017 mzz2017 mzz2017 approved these changes

Assignees

@piyoki piyoki

Labels
feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@piyoki @mzz2017