-
Notifications
You must be signed in to change notification settings - Fork 21
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
Improve java compatability #21
base: main
Are you sure you want to change the base?
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.
Approved. This makes sense why we want to do this. I think we should get @pyricau and a few others to weigh in.
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.
This is a very good change. Should we bump the major version number, is this breaking?
* Similar to kotlin's Function0<T> interface, or Supplier<T> from java, but compatible on all | ||
* android API levels and doesn't reference kotlin stdlib. |
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.
This is an implementation detail that belongs in the commit message, not the code.
* Similar to kotlin's Function0<T> interface, or Supplier<T> from java, but compatible on all | |
* android API levels and doesn't reference kotlin stdlib. | |
* Function to lazily provide a log message, if it is ever needed. |
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.
Good idea, I moved it to the commit message and used your suggestion instead.
Latest release is 0.1 . This can be for 0.2 ;) |
Technically yes this is a breaking change in the ABI, but it is source compatible based on the testing I did today |
aa0b258
to
2814cc5
Compare
public final fun getPriorityInt ()I | ||
public static fun valueOf (Ljava/lang/String;)Llogcat/LogPriority; | ||
public static fun values ()[Llogcat/LogPriority; | ||
} | ||
|
||
public final class logcat/LogcatKt { |
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.
This would be a source incompatible change for Java, but it's easy find/replace to fix it
public static final fun outerClassSimpleNameInternalOnlyDoNotUseKThxBye (Ljava/lang/Object;)Ljava/lang/String; | ||
} | ||
|
||
public abstract interface class logcat/LogcatLogger { | ||
public static final field Companion Llogcat/LogcatLogger$Companion; | ||
public static fun install (Llogcat/LogcatLogger;)V |
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.
Added @JvmStatic
a few places so you don't have to dig through the .Companion
object in java to get to these methods.
* Function to lazily provide a log message, if it is ever needed. | ||
*/ | ||
fun interface MessageSupplier { | ||
fun get(): String |
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.
nit: could probably use a better method name than get()
?
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.
Sure, updated
94ddd93
to
0dba23e
Compare
Adds a new MessageSupplier interface, which is similar to kotlin's Function0<T> interface, or Supplier<T> from java, but compatible on all android API levels and doesn't reference kotlin stdlib. This avoids having java consumers of this library getting flagged by dependency analysis plugin as using classes from kotlin-stdlib without declaring it as a dependency
* Get rid of ugly `*Kt` classnames in java * Make APIs @JvmStatic so you dont have to use the companion object from java source
0dba23e
to
bc31523
Compare
This avoids having java consumers of this library getting flagged by dependency analysis plugin as using classes from kotlin-stdlib without declaring it as a dependency.
Also updated the build tools while i was here :)