-
Notifications
You must be signed in to change notification settings - Fork 24
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
K2 plugin migration #321
base: master
Are you sure you want to change the base?
K2 plugin migration #321
Conversation
build.gradle.kts
Outdated
//tasks.named<RunIdeTask>("runIde") { | ||
// jvmArgumentProviders += CommandLineArgumentProvider { | ||
// listOf("-Didea.kotlin.plugin.use.k2=true") | ||
// } | ||
//} | ||
|
||
//tasks.test { | ||
// jvmArgumentProviders += CommandLineArgumentProvider { | ||
// listOf("-Didea.kotlin.plugin.use.k2=true") | ||
// } | ||
//} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe we should not commit out-commented code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without vm options, tests and debug IDE would be run against K1 mode. It's ok for 242 and below but for the 243+, it should be changed to something else. It's better to separate migration of the plugin and migration of the test infra indeed. Hopefully, it would be easy for the maintainers to find corresponding options without the comments.
@@ -21,51 +21,6 @@ class SpecTests : LightJavaCodeInsightFixtureTestCase() { | |||
return false | |||
} | |||
|
|||
fun testIsContainedInSpecFunSpec() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of merging `master´, rebase to keep the PR's history clean of merge commits for more convenient review.
- starting from 242, the plugin works over AnalysisAPI which hides all details about kotlin engine used inside kotlin plugin itself
Evident usages of K1 api (resolve) are migrated