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

Workaround non-null parent requirement of StubBase #513

Closed
wants to merge 1 commit into from

Conversation

jsjeon
Copy link
Contributor

@jsjeon jsjeon commented Aug 27, 2024

No description provided.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 27, 2024
@jsjeon jsjeon changed the title Upgrade to Kotlin 2.0.20 (#495) Upgrade to Kotlin 2.0.20 Aug 27, 2024
Copy link

@liutikas liutikas left a comment

Choose a reason for hiding this comment

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

Clever workaround!

@jsjeon
Copy link
Contributor Author

jsjeon commented Aug 27, 2024

cc @hick209 as per #495 history

Copy link
Contributor

@hick209 hick209 left a comment

Choose a reason for hiding this comment

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

I really appreciate the code you put here, but I wonder if we need to update the Kotlin version.

My worry is that if we put Kotlin 2.0.20 here then maybe people trying to use ktfmt with an earlier version of Kotlin will have issues integrating with it.
I understand that running ktfmt from a binary should not be problem, but there are instances where it is used as a code integration, like in LSPs or running it as a deamon.

core/pom.xml Outdated Show resolved Hide resolved
@jsjeon jsjeon changed the title Upgrade to Kotlin 2.0.20 Workaround non-null parent requirement of StubBase Aug 27, 2024
@hick209
Copy link
Contributor

hick209 commented Aug 27, 2024

nit: would you mind adding the change to the CHANGELOG.md?

@jsjeon
Copy link
Contributor Author

jsjeon commented Aug 27, 2024

nit: would you mind adding the change to the CHANGELOG.md?

I assumed it would be under Unreleased / fixed ?

@facebook-github-bot
Copy link
Contributor

@hick209 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@jsjeon
Copy link
Contributor Author

jsjeon commented Aug 27, 2024

Let me know what I need to do regarding Internal Linter error / test failures.

@hick209
Copy link
Contributor

hick209 commented Aug 28, 2024

@jsjeon it's just a minor thing, don't worry about it, I can take care of it 🙂

@facebook-github-bot
Copy link
Contributor

@hick209 merged this pull request in 9dcd261.

@simonhauck
Copy link

Hi,

Would it be possible for you to create a release with this workaround? I encountered projects where it is now not possible to update the Kotlin version and have proper formatting checks, since this issue is also propagated in the ktfmt-gralde plugin.

@stefanscheidt
Copy link

Hi, the integration via Spotless Gradle plugin is also affected.

@hick209
Copy link
Contributor

hick209 commented Oct 30, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Kotlin 2.0.20-Beta2: Exception when parsing a property accessor
6 participants