Skip to content

Refactor the logger #144

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

Merged
merged 7 commits into from
Jul 20, 2021

Conversation

swift-kim
Copy link
Member

  • Switch to C++-style streams from printf-like (C-style) function macros.
    • Inspired by the FML logger.
    • Introduce two new classes: Logger, LogMessage.
    • Avoid log level abbreviation, e.g. FT_LOGEFT_LOG(Error).
  • Clean up unused debug macros (FT_RELEASE_ASSERT, FT_COMPILE_ASSERT).
    • Instead, a new log level (Fatal) has been added which terminates the process immediately.
  • Remove some redundant logs.
  • Reword or simplify log messages for clarity and consistency.
  • Merge tizen_log.cc and tizen_log_stub.cc into one file.
  • Rename tizen_log.h to logger.h.
  • Do not print detailed EGL error messages, instead, use stringification to print enum names only.
  • Re-enable the --verbose-system-logs option by fixing typo in the arg name (verbose-logging--verbose-logging).

Sorry for another large commit. The change couldn't be divided into smaller commits.

Copy link
Member

@bwikbs bwikbs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like old style(printf-like) better.. but It looks like much more trendy code! 😄

@swift-kim
Copy link
Member Author

@bwikbs I also find the printf-style logs are more readable and easy to format in some cases, but the stream style has some benefits as explained in the Google style guide.

The << and >> stream operators provide an API for formatted I/O that is easily learned, portable, reusable, and extensible. printf, by contrast, doesn't even support std::string, to say nothing of user-defined types, and is very difficult to use portably. printf also obliges you to choose among the numerous slightly different versions of that function, and navigate the dozens of conversion specifiers.

I guess the portability here means the need to use format specifiers like %zu and macros like PRId64 for compatibility between 32-bit and 64-bit machines.

@bwikbs
Copy link
Member

bwikbs commented Jul 15, 2021

@swift-kim 👍 I can't say a word..

@swift-kim swift-kim added the no merge This PR is not ready for merge yet label Jul 15, 2021
@swift-kim
Copy link
Member Author

@bbrto21 I changed the log level of some messages in text_input_channel.cc from Info to Debug during the rebase, because there were too many logs printed out to the console. Are you okay with it?

@swift-kim swift-kim removed the no merge This PR is not ready for merge yet label Jul 20, 2021
@swift-kim swift-kim merged commit dce70c1 into flutter-tizen:flutter-2.2.1-tizen Jul 20, 2021
swift-kim added a commit that referenced this pull request Sep 27, 2021
* Rewrite tizen_log in the C++ style

* Convert logs to streams

* Rename tizen_log to logger

* Misc fixes

- Replace __FILE__ with __MODULE__.
- Resolve unused variable warning: kLogTag.
- Typo fix and etc.

* Change min logging level to Debug when --verbose-logging is set

* Resolve conflicts & additional clean-up

* Resolve conflicts again
swift-kim added a commit that referenced this pull request Nov 14, 2021
* Rewrite tizen_log in the C++ style

* Convert logs to streams

* Rename tizen_log to logger

* Misc fixes

- Replace __FILE__ with __MODULE__.
- Resolve unused variable warning: kLogTag.
- Typo fix and etc.

* Change min logging level to Debug when --verbose-logging is set

* Resolve conflicts & additional clean-up

* Resolve conflicts again
swift-kim added a commit that referenced this pull request Dec 9, 2021
* Rewrite tizen_log in the C++ style

* Convert logs to streams

* Rename tizen_log to logger

* Misc fixes

- Replace __FILE__ with __MODULE__.
- Resolve unused variable warning: kLogTag.
- Typo fix and etc.

* Change min logging level to Debug when --verbose-logging is set

* Resolve conflicts & additional clean-up

* Resolve conflicts again
swift-kim added a commit that referenced this pull request Dec 17, 2021
* Rewrite tizen_log in the C++ style

* Convert logs to streams

* Rename tizen_log to logger

* Change min logging level to Debug when --verbose-logging is set

* Resolve conflicts & additional clean-up
swift-kim added a commit that referenced this pull request Feb 7, 2022
* Rewrite tizen_log in the C++ style

* Convert logs to streams

* Rename tizen_log to logger

* Change min logging level to Debug when --verbose-logging is set

* Resolve conflicts & additional clean-up
swift-kim added a commit that referenced this pull request Feb 11, 2022
* Rewrite tizen_log in the C++ style

* Convert logs to streams

* Rename tizen_log to logger

* Change min logging level to Debug when --verbose-logging is set

* Resolve conflicts & additional clean-up
swift-kim added a commit that referenced this pull request May 12, 2022
* Rewrite tizen_log in the C++ style

* Convert logs to streams

* Rename tizen_log to logger

* Change min logging level to Debug when --verbose-logging is set

* Resolve conflicts & additional clean-up
swift-kim added a commit that referenced this pull request Aug 5, 2022
* Rewrite tizen_log in the C++ style

* Convert logs to streams

* Rename tizen_log to logger

* Change min logging level to Debug when --verbose-logging is set

* Resolve conflicts & additional clean-up
swift-kim pushed a commit that referenced this pull request Sep 1, 2022
Speculative fixes for Windows build
swift-kim added a commit that referenced this pull request Sep 1, 2022
* Rewrite tizen_log in the C++ style

* Convert logs to streams

* Rename tizen_log to logger

* Change min logging level to Debug when --verbose-logging is set

* Resolve conflicts & additional clean-up
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

Successfully merging this pull request may close these issues.

3 participants