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

make annotations-optional a KMP library #81

Merged
merged 7 commits into from
Oct 31, 2024

Conversation

gabrielittner
Copy link

With this ForScope and SingleIn can be used with both Dagger and kotlin-inject. All targets have the kotlin-inject annotations and the jvm target has the javax.inject annotations like before (I've also added jakarta while I was at it).

Comment on lines +53 to +54
# TODO: remove when updating to Kotlin 2.0
kotlin.native.ignoreIncorrectDependencies=true
Copy link
Author

Choose a reason for hiding this comment

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

Kotlin 1.x will warn on common compileOnly dependencies even if they are added to the non jvm source sets, 2.0 resolved that.

@gabrielittner
Copy link
Author

Ah, it looks like kotlin-inject with mingw_x64 support doesn't have a release yet. I could revert the last commit and we go with just making the annotations available in KMP.

@ZacSweers
Copy link
Owner

yeah just revert the windows support for now. Can add it back when kotlin-inject gets around to another release

@ZacSweers
Copy link
Owner

Can you add a note to the changelog too?

@gabrielittner
Copy link
Author

Removed windows and added changelog 👍

@ZacSweers
Copy link
Owner

@gabrielittner
Copy link
Author

6be98de should fix it

@gabrielittner
Copy link
Author

All green now. I'll open a small follow up after this is merged to make the publishing setup slightly nicer.

import me.tatarka.inject.annotations.Qualifier as KotlinInjectQualifier

/**
* A class based [qualfier](Qualifier).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* A class based [qualfier](Qualifier).
* A class based [qualifier](Qualifier).

Copy link
Author

Choose a reason for hiding this comment

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

@ZacSweers ZacSweers merged commit c957242 into ZacSweers:main Oct 31, 2024
16 checks passed
@gabrielittner gabrielittner deleted the optional-kmp branch November 1, 2024 07:48
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