RNET-1161: Implement support for using a log level for a specific log category #3634
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Pull Request Test Coverage Report for Build 9679342781Details
💛 - Coveralls |
elle-j
commented
Jun 26, 2024
Pull Request Test Coverage Report for Build 9681475484Details
💛 - Coveralls |
nirinchev
reviewed
Jun 26, 2024
nirinchev
reviewed
Jun 28, 2024
Pull Request Test Coverage Report for Build 9715427241Details
💛 - Coveralls |
Pull Request Test Coverage Report for Build 9742115832Details
💛 - Coveralls |
elle-j
commented
Jul 2, 2024
Pull Request Test Coverage Report for Build 9810938552Details
💛 - Coveralls |
clementetb
reviewed
Jul 8, 2024
nirinchev
reviewed
Jul 9, 2024
nirinchev
reviewed
Jul 10, 2024
Pull Request Test Coverage Report for Build 9894480239Details
💛 - Coveralls |
papafe
reviewed
Jul 10, 2024
| @@ -223,8 +236,14 @@ private static class NativeMethods | |||
| [DllImport(InteropConfig.DLL_NAME, EntryPoint = "shared_realm_refresh_async", CallingConvention = CallingConvention.Cdecl)] | |||
| public static extern bool refresh_async(SharedRealmHandle realm, IntPtr tcs_handle, out NativeException ex); | |||
|
|
|||
| [DllImport(InteropConfig.DLL_NAME, EntryPoint = "shared_realm_get_log_level", CallingConvention = CallingConvention.Cdecl)] | |||
| public static extern LogLevel get_log_level([MarshalAs(UnmanagedType.LPWStr)] string category_name, IntPtr category_name_len); | |||
There was a problem hiding this comment.
I think we're trying to go with using Realm.Native.StringValue in wrapper calls when needing to pass strings. Am I correct @nirinchev ?
There was a problem hiding this comment.
My understanding here was that StringValue is mainly used for the returned strings (Core -> DotNet), whereas the MarshalAs is DotNet -> Core primarily?
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| @@ -20,11 +20,11 @@ | |||
|
|
|||
| namespace UnityUtils | |||
| { | |||
| public class UnityLogger : Logger | |||
| public class UnityLogger : RealmLogger | |||
Check notice
Code scanning / CodeQL
Missing a summary in documentation comment Note
nirinchev
approved these changes
Jul 11, 2024
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Deprecates
Loggerin favor of its new base classRealmLogger(see notes below on avoiding breaking changes).Primarily, enables more control over which category of messages should be logged and at which criticality level, by setting and getting a
LogLevelfor a givenLogCategory. The hierarchy of categories starts atLogCategory.Realm.Adds a function logger that accepts a callback that will receive the
LogLevel,LogCategory, and the message when invoked.Adds a
RealmLogger.Log()overload taking a category.Notes on avoiding breaking changes
Would-be breaking changes
Users have been able to provide their own
Loggersubclass via theLogger.Defaultsetaccessor. Custom loggers need to override and implementLogger.LogImpl(), and with the addition of log categories in this PR, theLogImpl()signature would require breaking changes.Solution
Essentially, the previous
Loggerclass has been renamedRealmLogger, and another class calledLoggernow derives fromRealmLogger, with the only own member being theLogImpl()with an abstract method identical to before to avoid the breaking change.Fixes #3633
@realm/devdocs
TODO