Add runtime kotlin version detection for platform info purposes. #693
Conversation
| try { | ||
| return kotlin.KotlinVersion.CURRENT.toString(); | ||
| } catch (NoClassDefFoundError ex) { | ||
| return null; |
There was a problem hiding this comment.
Can we write this as "no_kotlin_version" or "-1". Could give us a sense of what portion of clients do not have kotlin. I realise this can be derived already. But, seems more convenient
There was a problem hiding this comment.
I erred on the side of not populating what's not present, e.g. it's consistent with what is done for unity, flutter, cpp, etc. So I think I prefer to do it this way, but @davidair could correct me if that's not the case and/or your suggestion adds convenience.
There was a problem hiding this comment.
We normally only put things into the platform info that "are there" - and they vary between platform. It doesn't make sense to put the Xcode version into the Android string, so I would recommend omitting Kotlin if not present.
firebase-common/src/main/java/com/google/firebase/FirebaseApp.java
Outdated
Show resolved
Hide resolved
This is done to minimize the scope of `-dontwarn` in the consumer proguard spec which is required for proguard to succeed in developers' apps. Note: the `-dontwarn` does not seem to be required when building the app with R8 instead of proguard.
| @@ -42,7 +42,7 @@ class VersionTests { | |||
| fun libraryVersions_shouldBeRegisteredWithRuntime() { | |||
| withApp("ktxTestApp") { | |||
| val uaPublisher = get(UserAgentPublisher::class.java) | |||
| assertThat(uaPublisher.userAgent).contains("kotlin") | |||
| assertThat(uaPublisher.userAgent).contains("fire-kotlin") | |||
There was a problem hiding this comment.
Please keep as "kotlin" since it's not a Firebase product
No description provided.