-
Notifications
You must be signed in to change notification settings - Fork 16
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
LogHandler removal & logging improvements #340
Conversation
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.
Just a thought. What do you / others think?
9ca4ad8
to
19de88c
Compare
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.
Thanks. Looks good, just a couple of tweaks before you merge this, please...
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.
Quickly scanned through changes, all looks good to me
This should resolve #238, although it does not use the mentioned
LogHandler
. Instead, the changes contain:LogHandler
entirely from Dart codeLog
implementation fromably-java
. Luckily, their implementation is the same so it was only a matter of importsprint
andNSLog
calls withARTLog
implementation fromably-cocoa
. This was not possible everywhere, because unlikeLog
fromably-java
theARTLog
isn't a singleton, so I only updated a few placesWith these changes, most of the logging from Android and iOS platform native code should respect logging level set in Ably
ClientOptions
.