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 macros #1305

Closed
acelyc111 opened this issue Jan 10, 2023 · 8 comments
Closed

Refactor log macros #1305

acelyc111 opened this issue Jan 10, 2023 · 8 comments
Assignees

Comments

@acelyc111
Copy link
Member

There are two type of log macros in the code, one is using C string format specifiers, like LOG_INFO, the other is using libfmt, like LOG_INFO_F.

It's strange to keep the two macros in long term, I'm planning to refactor them and only leave libfmt style one.

@empiredan
Copy link
Contributor

After all LOG_* macros have been replaced with LOG_*_F, would LOG_*_F be renamed as LOG_*? For example, LOG_INFO_F is renamed as LOG_INFO ?

@acelyc111
Copy link
Member Author

After all LOG_* macros have been replaced with LOG_*_F, would LOG_*_F be renamed as LOG_*? For example, LOG_INFO_F is renamed as LOG_INFO ?

Yes, I'm planning to still use LOG_*_F before all refactor work been finished, then remove _F postfix at last in one patch.

@acelyc111
Copy link
Member Author

After all LOG_* macros have been replaced with LOG_*_F, would LOG_*_F be renamed as LOG_*? For example, LOG_INFO_F is renamed as LOG_INFO ?

Yes, I'm planning to still use LOG_*_F before all refactor work been finished, then remove _F postfix at last in one patch.

In this way, all macros with or without _F postfix are both in the same semantics in the progress.

@empiredan
Copy link
Contributor

After all LOG_* macros have been replaced with LOG_*_F, would LOG_*_F be renamed as LOG_*? For example, LOG_INFO_F is renamed as LOG_INFO ?

Yes, I'm planning to still use LOG_*_F before all refactor work been finished, then remove _F postfix at last in one patch.

In this way, all macros with or without _F postfix are both in the same semantics in the progress.

I think it is important to keep all macros consistent without intermediate stages. Let's go ahead !

@Smityz
Copy link
Contributor

Smityz commented Jan 14, 2023

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

@acelyc111
Copy link
Member Author

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

I assigned this work to you, you can implement it if you have time.

@acelyc111
Copy link
Member Author

acelyc111 commented Jan 16, 2023

@GehaFearless Could you please to do the remain work?

  • LOG_WARN

    • replica module
    • meta module
    • others
  • LOG_ERROR

    • replica module
    • others

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

No branches or pull requests

3 participants