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

Replace .form UI files with Kotlin UI DSL v2 #554

Merged
merged 4 commits into from
Feb 9, 2024

Conversation

jansorg
Copy link
Collaborator

@jansorg jansorg commented Feb 2, 2024

Closes #548

Replaces the .form files with UI DSL v2. Support for .form files is deprecated and mostly unmaintained by JetBrains now.

Previously, code instrumentation via the IntelliJ Gradle plugin was used to create .class files from the JetBrains .form files. Instrumentation is not needed anymore and is disabled by this PR.

UI DSL v2 needs Kotlin. This PR enables Kotlin to allow writing the UI DSL code, but everything else is going to remain Java source code.

Unfortunately, the UI DSL isn't backwards compatible in a few places. We're dropping support for 2021.3 to fix this.

Settings:
image

Start AppMap recording:
image

Stop AppMap recording:
image

@jansorg jansorg requested a review from ahtrotta February 2, 2024 15:14
@jansorg jansorg self-assigned this Feb 2, 2024
@jansorg jansorg force-pushed the jansorg/replace-ui-forms branch from b4b8e9d to f3076c4 Compare February 2, 2024 15:15
Copy link
Contributor

@ahtrotta ahtrotta left a comment

Choose a reason for hiding this comment

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

When I run the plugin locally, I get this warning related to this change:

> Task :plugin-core:compileKotlin
w: Lombok Kotlin compiler plugin is an experimental feature. See: https://kotlinlang.org/docs/components-stability.html.
w: file:///home/ahtrotta/projects/appmap/appmap-intellij-plugin/plugin-core/src/main/kotlin/appland/remote/StartRemoteRecordingForm.kt:22:22 'horizontalAlign(HorizontalAlign): Cell<T>' is deprecated. Use align(AlignX.LEFT/CENTER/RIGHT/FILL) method instead
w: file:///home/ahtrotta/projects/appmap/appmap-intellij-plugin/plugin-core/src/main/kotlin/appland/remote/StopRemoteRecordingForm.kt:18:60 'horizontalAlign(HorizontalAlign): Cell<T>' is deprecated. Use align(AlignX.LEFT/CENTER/RIGHT/FILL) method instead
w: file:///home/ahtrotta/projects/appmap/appmap-intellij-plugin/plugin-core/src/main/kotlin/appland/remote/StopRemoteRecordingForm.kt:25:43 'horizontalAlign(HorizontalAlign): Cell<T>' is deprecated. Use align(AlignX.LEFT/CENTER/RIGHT/FILL) method instead

Could you address this? Thanks!

@jansorg
Copy link
Collaborator Author

jansorg commented Feb 6, 2024

@ahtrotta Is it possible that you were compiling for a later SDK?
I'm not seeing these warnings with the default platform version, but I know that later versions added deprecations like this.

@jansorg jansorg requested a review from ahtrotta February 6, 2024 15:31
@jansorg jansorg force-pushed the jansorg/replace-ui-forms branch from 422e3a9 to 4972c3d Compare February 6, 2024 15:31
@jansorg jansorg marked this pull request as draft February 6, 2024 15:32
@jansorg jansorg removed the request for review from ahtrotta February 6, 2024 15:33
@ahtrotta
Copy link
Contributor

ahtrotta commented Feb 6, 2024

@ahtrotta Is it possible that you were compiling for a later SDK? I'm not seeing these warnings with the default platform version, but I know that later versions added deprecations like this.

Yeah I was using 2023.3.2

@jansorg jansorg force-pushed the jansorg/replace-ui-forms branch from 4972c3d to 2f55663 Compare February 7, 2024 09:48
Copy link

github-actions bot commented Feb 7, 2024

AppMap runtime code review

Summary Status
Failed tests ✅ All tests passed
API changes 0️⃣ No API changes
Security flaws ✅ None detected
Performance problems ✅ None detected
Code anti-patterns ✅ None detected
New AppMaps ⭐ 1 new junit test
Removed AppMaps ✖️ 1 removed junit test

⭐ New AppMaps

[junit] Findings manager modification dates from plugin-core/src/test/java/appland/problemsView/FindingsManagerTest.java:69

✖️ Removed AppMaps

[junit] App map search scopes app map config scope

@jansorg jansorg force-pushed the jansorg/replace-ui-forms branch 5 times, most recently from 64daaba to fe2489f Compare February 7, 2024 14:43
@ahtrotta
Copy link
Contributor

ahtrotta commented Feb 8, 2024

@jansorg what is the status of this PR?

@jansorg
Copy link
Collaborator Author

jansorg commented Feb 8, 2024

There's a flaky test, but only on CI. I'll fix it soon.
It's not a user-visible change, so it's not important for UX/UI testing.

@ahtrotta
Copy link
Contributor

ahtrotta commented Feb 8, 2024

There's a flaky test, but only on CI. I'll fix it soon. It's not a user-visible change, so it's not important for UX/UI testing.

Thanks for the update! I was just wondering because the issue is in the In Review column on the board but it's marked as a draft PR. I'll move the issue to In Progress.

@jansorg
Copy link
Collaborator Author

jansorg commented Feb 8, 2024

Oh, sorry! I thought it was ready, but missed the test.

@jansorg jansorg force-pushed the jansorg/replace-ui-forms branch from fe2489f to 488a5f6 Compare February 9, 2024 09:17
@jansorg jansorg force-pushed the jansorg/replace-ui-forms branch 2 times, most recently from 67a1f0c to 7b3ae77 Compare February 9, 2024 09:28
@jansorg jansorg force-pushed the jansorg/replace-ui-forms branch from 7b3ae77 to e7bdb64 Compare February 9, 2024 09:44
@jansorg jansorg marked this pull request as ready for review February 9, 2024 09:46
@jansorg jansorg requested a review from ahtrotta February 9, 2024 10:03
@jansorg
Copy link
Collaborator Author

jansorg commented Feb 9, 2024

When I run the plugin locally, I get this warning related to this change:

> Task :plugin-core:compileKotlin
w: Lombok Kotlin compiler plugin is an experimental feature. See: https://kotlinlang.org/docs/components-stability.html.
w: file:///home/ahtrotta/projects/appmap/appmap-intellij-plugin/plugin-core/src/main/kotlin/appland/remote/StartRemoteRecordingForm.kt:22:22 'horizontalAlign(HorizontalAlign): Cell<T>' is deprecated. Use align(AlignX.LEFT/CENTER/RIGHT/FILL) method instead
w: file:///home/ahtrotta/projects/appmap/appmap-intellij-plugin/plugin-core/src/main/kotlin/appland/remote/StopRemoteRecordingForm.kt:18:60 'horizontalAlign(HorizontalAlign): Cell<T>' is deprecated. Use align(AlignX.LEFT/CENTER/RIGHT/FILL) method instead
w: file:///home/ahtrotta/projects/appmap/appmap-intellij-plugin/plugin-core/src/main/kotlin/appland/remote/StopRemoteRecordingForm.kt:25:43 'horizontalAlign(HorizontalAlign): Cell<T>' is deprecated. Use align(AlignX.LEFT/CENTER/RIGHT/FILL) method instead

Could you address this? Thanks!

@ahtrotta I've fixed the test, this PR is ready for review now.
Because the above API was only added in a later version of the SDK, we're unable to fix it.

@ahtrotta
Copy link
Contributor

ahtrotta commented Feb 9, 2024

@ahtrotta I've fixed the test, this PR is ready for review now. Because the above API was only added in a later version of the SDK, we're unable to fix it.

Okay, great I'll take a look!

@ahtrotta ahtrotta merged commit 3679dcd into develop Feb 9, 2024
6 checks passed
@ahtrotta ahtrotta deleted the jansorg/replace-ui-forms branch February 9, 2024 14:53
@appland-release
Copy link
Contributor

🎉 This PR is included in version 0.56.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

Replace .form UI files with code-based UI
3 participants