Skip to content

Prevent K2 warning from showing for KSP users - #98

Merged
ZacSweers merged 1 commit into
ZacSweers:mainfrom
WhosNickDoglio:ndoglio/kotlin-warning
May 1, 2026
Merged

ZacSweers merged 1 commit into
ZacSweers:mainfrom
WhosNickDoglio:ndoglio/kotlin-warning

Conversation

@WhosNickDoglio

@WhosNickDoglio WhosNickDoglio commented Mar 13, 2026 •

Copy link
Copy Markdown

This PR was AI assisted

The main changes we're updating isApplicable to return false when KSP was enabled, there was some stuff in applyToCompilation that needed to be moved to the apply function as they also applicable for when KSP was enabled.

override fun applyToCompilation(
kotlinCompilation: KotlinCompilation<*>,
): Provider<List<SubpluginOption>> {
kotlinCompilation.compilerOptions.options.let {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

We should just not do anything here if KSP is being used? I thought that was already the case but seems I misremembered. Is there any reason for the compiler plugin to be configured here?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Given the state of the library I was trying to keep it small but happy to remove all of this if that's what you'd prefer. AFAIK there's no reason for any of this to stick around. 👍

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

let's just have it return early if it's only in KSP mode here, nothing toooo invasive

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Understood! Can do 👍

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@ZacSweers this should be ready for review now 🫡

@WhosNickDoglio
WhosNickDoglio force-pushed the ndoglio/kotlin-warning branch 2 times, most recently from 369b65d to e7698e3 Compare March 16, 2026 20:12
@WhosNickDoglio
WhosNickDoglio marked this pull request as draft March 16, 2026 20:20
@WhosNickDoglio
WhosNickDoglio force-pushed the ndoglio/kotlin-warning branch from e7698e3 to 92950d7 Compare March 20, 2026 20:48
@ZacSweers

Copy link
Copy Markdown
Owner

Is this ready for review? Noticed it's still in draft but not sure if you're waiting for me

@WhosNickDoglio

Copy link
Copy Markdown
Author

Sorry still working on this! I got pulled into some other work but will be coming back to this this week.

It passes CI here but trying to test in our main repo led to some weird failures so I'm debugging that now. Once this is ready for review I'll move it out of draft and let you know 👍

Thank you for maintaining this project, I promise we're working on migrating to Metro sometime this year 😅

@WhosNickDoglio
WhosNickDoglio force-pushed the ndoglio/kotlin-warning branch 2 times, most recently from 72d46c4 to fe5e006 Compare April 29, 2026 21:34
@WhosNickDoglio
WhosNickDoglio force-pushed the ndoglio/kotlin-warning branch from fe5e006 to d8e9d83 Compare April 29, 2026 21:42
@WhosNickDoglio WhosNickDoglio changed the title Remove K2 warning that's not relevant for KSP users Prevent K2 warning from showing for KSP users Apr 29, 2026
@WhosNickDoglio
WhosNickDoglio marked this pull request as ready for review April 29, 2026 22:07
@ZacSweers
ZacSweers merged commit 8f22fdb into ZacSweers:main May 1, 2026
15 checks passed
@ZacSweers

Copy link
Copy Markdown
Owner

Any guidance we need to include in a changelog for this?

@WhosNickDoglio

Copy link
Copy Markdown
Author

Any guidance we need to include in a changelog for this?

No I don't think so, no changes necessary on the user's end, just the warning going away. 👍

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.

2 participants