TIMOB-9306: Blackberry: add a universal logging mechanism for tibb #85
Conversation
[Issues Fixed] TIMOB-9306: Blackberry: add a universal logging mechanism for tibb [Added] NativeLoggerInterface.h NativeLoggerObject.cpp NativeLoggerObject.h NativeLoggerWorker.cpp NativeLoggerWorker.h TiLogger.cpp TiLogger.h [Changes] NativeAbstractTextControlObject.cpp - switched to use N_DEBUG NativeObject.h - added N_TYPE_LOGGER - removed old Ti_DEBUG NativeObjectFactory.cpp - added N_TYPE_LOGGER TiUITableView.cpp - switched to use Ti_DEBUG TitaniumRuntime.cpp - initialize TiLogger [Tests] Test 1: Use logger 1) Put some Ti_DEBUG and N_DEBUG statements in the code that will be run 2) Build tibb and verify there are no errors 3) Create, build, and run a new project 4) Verify the logging statements are in the app's log and are not intermingled
|
|
||
| void TiLogger::log(std::string msg) | ||
| { | ||
| nativeLogger_->log(msg.c_str()); |
There was a problem hiding this comment.
please check the pointer for null-ness
There was a problem hiding this comment.
If nativeLogger_ is null then you didn't call initialize() first or the factory is broken so a crash is appropriate.
There was a problem hiding this comment.
If the initialize is not called, I would prefer to miss logs (by checking against null-ness) rather than to make app crash
There was a problem hiding this comment.
As discussed, in the case it makes more sense to crash.
|
reviewed |
Disables copy ctor and assignment operator, plus added comment.
|
Updated patch |
| TiLogger(const TiLogger&); | ||
| TiLogger& operator=(const TiLogger&); | ||
|
|
||
| static NativeLoggerInterface* nativeLogger_; |
|
The logger should allow for TAGS (INFO, DEBUG, WARNING, ERROR, INTERNAL) logged in a way that makes them consistant so the logs can be filtered on them. Could have a X_LOG that does what X_DEBUG currently does and take a TAG/severity param and have a MACRO for each of the TAGS (X_INFO, X_DEBUG, X_WARNING, X_ERROR, X_INTERNAL) so we don't have to specify the tag as a param |
|
|
||
| void NativeLoggerWorker::log(const QString& t) | ||
| { | ||
| fprintf(stderr, "%s", t.toStdString().c_str()); |
There was a problem hiding this comment.
Are all logs errors? Can we have log flags where some logs go to stderr, some go to stdout, and some are stored in a file? This would be useful for testing release versions of the software where stderr and stdout would not be visible.
There was a problem hiding this comment.
I don't see why stdout would be appropriate since these aren't outputs resulting from running the app, they're just logs. I don't think it makes a difference though because I see the logs in QDE and the logs/log file on the device. In other words, stdout and stderr both seem to go to the same places.
|
reviewed |
|
approved |
|
Approved |
Changed to use streams instead for easier appending Added tagged macros Changed naming of s_nativeLogger and TI_DEBUG
|
@jpl-mac I've added the tagged macros and also changed it to use streams since it's able to handle more things than std::string's + operator. |
|
Updated patch |
|
Thanks David, could you use this opportunity to also move the debug strings from NativeAbstractTextControlObject.cpp to TiMessageStrings or a native equivalent? |
[Added] NativeMessageStrings.cpp NativeMessageStrings.h [Changed] Moved debug strings to central location Removed unused function TitaniumRuntime::Log
|
Updated patch: moved debug strings to central location |
|
Thanks David |
Conflicts: blackberry/tibb/NativeObjectFactory.cpp blackberry/tibb/NativeStringInterface.cpp blackberry/tibb/TiMessageStrings.h blackberry/tibb/TiRootObject.cpp
TIMOB-9306: Blackberry: add a universal logging mechanism for tibb [Issues Fixed] TIMOB-9306: Blackberry: add a universal logging mechanism for tibb [Added] NativeLoggerInterface.h NativeLoggerObject.cpp NativeLoggerObject.h NativeLoggerWorker.cpp NativeLoggerWorker.h NativeMessageStrings.cpp NativeMessageStrings.h TiLogger.cpp TiLogger.h [Changes] NativeAbstractTextControlObject.cpp - switched to use N_DEBUG NativeObject.h - added N_TYPE_LOGGER - removed old Ti_DEBUG NativeObjectFactory.cpp - added N_TYPE_LOGGER TiUITableView.cpp - switched to use Ti_DEBUG TitaniumRuntime.cpp - initialize TiLogger Moved debug strings to central location Removed unused function TitaniumRuntime::Log Also moved the Native strings into NativeMessageStrings.h [Tests] Test 1: Use logger 1) Put some Ti_DEBUG and N_DEBUG statements in the code that will be run 2) Build tibb and verify there are no errors 3) Create, build, and run a new project 4) Verify the logging statements are in the app's log and are not intermingled
Reviewers: David C, Harut