-
Notifications
You must be signed in to change notification settings - Fork 22
Remove JVM specific API usage in common code #37
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
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.
Nice changes!
Adding non-JVM targets will result in compilation failure
"It is recommended to report your use-case of internal API to logcat issue tracker, " + | ||
"so stable API could be provided instead" | ||
) | ||
annotation class InternalLogcatApi |
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.
why add this? Why was the internal
keyword not enough?
@@ -63,9 +66,9 @@ inline fun Any.logcat( | |||
} | |||
val loggers = LogcatLogger.loggers.filter { it.isLoggable(priority) } | |||
if (loggers.isNotEmpty()) { | |||
val tagOrCaller = tag ?: outerClassSimpleNameInternalOnlyDoNotUseKThxBye() |
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.
The name outerClassSimpleNameInternalOnlyDoNotUseKThxBye()
was picked so that Java code, which can see these methods, would not attempt to use it. Why rename?
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.
I was under the assumption that the InternalLogcatApi
annotation will work on Java side. Turns out it doesn't
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.
Adding non-JVM targets will result in compilation failure