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
[ios] add default OSLog Logger for ios>14.0 to release #8040
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Kiryl Kaveryn <kirylkaveryn@gmail.com>
|
||
if #available(iOS 14.0, *), enableLoggingInRelease { | ||
os.Logger.logger.log(level: OSLogLevelFromLogLevel(level), "\(formattedMessage, privacy: .public)") | ||
} else if Logger.canLog(level) { | ||
Logger.log(level, message: formattedMessage) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it make sense to also log into this logger on iOS 14+?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are using this logger for swift to pass it to the core logger. But there only 2 places where the logging from swift is used now.
I don't think that it's useful to duplicate logging messages. But of them will be visible in the debugger.
@@ -27,18 +28,42 @@ func statusBarHeight() -> CGFloat { | |||
return min(statusBarSize.height, statusBarSize.width) | |||
} | |||
|
|||
private let enableLoggingInRelease = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
static?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In swift variables outside the types are already static. Compiler will warn if I add the satatic
.
What is the current status here? @biodranik do you have any other comments? |
This PR creates a separate independent logger from our existing C++ logs engine which logs only a few specific cases (now only iCloud). It would be great to:
|
This PR adds logging using the default OSLogger for ios>14.0 during the release.
This PR does not affect the core logging.