Skip to content
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

refactor(log): use LOG_INFO_F instead of LOG_INFO (3/3) #60

Closed
wants to merge 3 commits into from

Conversation

acelyc111
Copy link
Owner

@acelyc111 acelyc111 commented Jan 13, 2023

apache#1305

This patch aim to use LOG_INFO_F instead of LOG_INFO for all the remain parts:

  • Use LOG_INFO_F instead of LOG_INFO
  • Use LOG_INFO_PREFIX instead of LOG_INFO for class proxy_session, pegasus_server_impl
  • Use upper case for commands in class redis_parser
  • Remove some commentted LOG_INFO code

TODO:

  1. print dsn_primary_address in log is useless, since it must be itself in whose log, e.g. in src/common/fs_manager.cpp, src/replica/replica_stub.cpp
  2. try to use LOG_*_PREFIX for class policy_context, cold_backup_context

@github-actions github-actions bot added the cpp label Jan 13, 2023
@acelyc111 acelyc111 changed the title Log info other refactor(log): use LOG_INFO_F instead of LOG_INFO (3/3) Jan 13, 2023
@github-actions github-actions bot changed the title refactor(log): use LOG_INFO_F instead of LOG_INFO (3/3) Log info other Jan 13, 2023
@github-actions github-actions bot changed the title Log info other refactor(log): use LOG_INFO_F instead of LOG_INFO (3/3) Jan 13, 2023
@Smityz
Copy link

Smityz commented Jan 14, 2023

Meticulous work! And LGTM!
Can we add some checking of parameter count in the log function?
ref: fmtlib/fmt#2593

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants