WycliffeAssociates / jdenticon-kotlin

19 stars 8 forks source link

Using lib in Kotlin/JS projects #4

Open ShirinkinArseny opened 4 years ago

ShirinkinArseny commented 4 years ago

Hi!

I'd like to use your library in Kotlin/JS project, however you build it only for JVM target.

The only file that uses some JVM-specifics is IconGenerator.kt: it imports java.util.Arrays.asList, but it can be replaced with kotlin's listOf().

I can add PR with new build.gradle, that will have two targets (Kotlin/JVM and Kotlin/JS), but there will be two issues:

  1. Artifact id will change. There will be three artifacts:
    • jdenticon-kotlin-jvm for using in JVM environments,
    • jdenticon-kotlin-js for JS envs,
    • jdenticon-kotlin as a common artifact for using in common modules (that are for multiple targets themselfs).

It can cause troubles, because of version 1.x in your readme (jvm projects that will be built with new version will stop working, users will have to change dependency name). It can be avoided by releasing version 2.*

  1. I am not sure if publishing will be working or not, because there will be three artifacts as a result of building, not one.

Thanks.

jsarabia commented 3 years ago

I apologize for not seeing this earlier! I have no experience with kotlin multiplatform, and porting the original javascript library was kind of an excuse for me originally to start learning kotlin, so I'm sorry for the unnecessary use of a java platform dependency!

If you are still interested in making this change, I would certainly welcome it, though I will warn you that issue #5 I would consider to be a bug fix and I'm not sure yet how that will be resolved and might bring in an additional java dependency.

bseib commented 3 years ago

@ShirinkinArseny that would be interesting to see what Kotlin would generate for the javascript output and how big it would be. I just added code that uses MessageDigest. Do you know if that exists as a Multiplatform lib?

ShirinkinArseny commented 3 years ago

As I know, there is no MessageDigest in standart library.

But there is option to define function as 'expect', and then define 'actual' implementations specific to each platform (https://kotlinlang.org/docs/reference/mpp-connect-to-apis.html). So we can:

But, once again, there will be three different modules, and it can potentially break build of existing projects that use this library. Users will have to change jdenticon-kotlin to jdenticon-kotlin-jvm in their pom/build.gradle files. Does anybody knows the way to eliminate this problem, or we can just ignore it?

I'll try to provide a demo within a week.

jsarabia commented 3 years ago

@ShirinkinArseny okio seems to have a KMP alternative for sha1 https://github.com/square/okio/blob/master/okio/src/commonMain/kotlin/okio/HashingSource.kt

I think it's probably fine to change the module to jdenticon-kotlin-jvm. I'll just make sure it's clearly communicated in the README and close to the top. It won't affect existing usage if people don't update.

jsarabia commented 3 years ago

I have merged your pull request, @ShirinkinArseny, (thank you for the contribution!!) and tried to publish it. I went ahead and published the previous update (fixing the hash) as version 1.1 before uploading this one (as 1.2).

The files were uploaded and I can download them from the bintray dashboard, but for whatever reason I get a 404 when trying to get it from within gradle. I'll have to look into this issue later, but I did verify that the code compiles and the output is as expected.

ShirinkinArseny commented 3 years ago

@jsarabia

Official jcenter (https://jcenter.bintray.com/org/wycliffeassociates/) does not contains '1.2' version.

However, project's repository (https://dl.bintray.com/bible-translation-tools/jdenticon-kotlin/org/wycliffeassociates/) includes 1.2 multiplatform packages.

I tried to add it as maven repo in build.gradle, and it worked:

repositories {
    maven {
        url("https://dl.bintray.com/bible-translation-tools/jdenticon-kotlin/")
    }
}
dependencies {
    implementation "org.wycliffeassociates:jdenticon-kotlin-jvm:1.2"
}

So I assume that artifacts are published, but there are some issues on the Bintray/Jcenter side. Maybe there are some extra verification/moderation steps if the artifacts names changed?