Skip to content
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

feat: android target #2

Merged
merged 3 commits into from
Feb 17, 2024
Merged

feat: android target #2

merged 3 commits into from
Feb 17, 2024

Conversation

sgpublic
Copy link

Hello, first of all thank you for your great work on Kotlin-Multiplatform-AppDirs.

Currently AppDir has completed all desktop targets. It would be great if the android target could be added. I have completed the Android target in my fork, so I created this PR.

Thank you!

Copy link
Owner

@Syer10 Syer10 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! A few comments but hopefully we can get this in

src/androidMain/kotlin/ca/gosyer/appdirs/impl/Util.kt Outdated Show resolved Hide resolved
build.gradle.kts Outdated Show resolved Hide resolved
build.gradle.kts Outdated Show resolved Hide resolved
Comment on lines 16 to 23
private var _Context: WeakReference<Context>? = null
internal val ContextRef: Context get() {
return _Context?.get() ?: throw AppDirsException("In android target, please call Application#attachAppDirs() first!")
}

fun Application.attachAppDirs() {
_Context = WeakReference(this)
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well done! This is the main thing I was worried about when thinking of how to implement a Android target. This seems like a good solution

@@ -28,6 +25,14 @@ kotlin {
linuxArm64()
mingwX64()

androidTarget {
publishLibraryVariants("release")
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way, the reason for adding this line is a bug from gradle-maven-publish-plugin

@elonzh
Copy link

elonzh commented Feb 4, 2024

@Syer10 Can you merge this nice PR?

Copy link
Owner

@Syer10 Syer10 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll fix the actions myself

@Syer10 Syer10 merged commit 82d3d5d into Syer10:master Feb 17, 2024
1 of 2 checks passed
@Syer10
Copy link
Owner

Syer10 commented Feb 17, 2024

Thanks! Its included in the latest release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants