-
-
Notifications
You must be signed in to change notification settings - Fork 40
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
Prepare for IntelliJ 2024.2 cycle #195
Conversation
26312c6
to
7cadb34
Compare
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.
Thanks for your work on this @unshare. I see that this is still in Draft. Do you think that it is ready for merging? All the tests look OK and the changes are also OK.
A the moment I'm awaiting JetBrains/intellij-plugin-verifier#1097 to be merged. A better approach would be migrate to "new" (2.x) IntelliJ Platform Plugin SDK, but it's too much for the time being. Merging without working 2024.2 support kind of defeats the purpose of this PR. |
4e1cf2b
to
881e0dd
Compare
@filiphr Look, I'm doing a spring cleanup for all the stuff I personally deem necessary at the moment. Given I'm the one who did the cleanup around Gradle the last time 1.5 years ago, it looks like I'm the only one who cares about Gradle. Kind of makes me maniac. As for everything else, it looks like tests pass with a certain Please, don't hesitate to tell me to piss off with the changes I propose, but please do so granularly and reasonably. And yes, this is still a draft. |
If you're okay with switching from Gradle Groovy DSL to Kotlin DSL, just give me a go ahead. |
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.
@filiphr Look, I'm doing a spring cleanup for all the stuff I personally deem necessary at the moment.
When the list of changes is ~complete, I'd update both commit message and PR description as well.
The PR will be un-drafted as well.
That is completely fine with me. I mainly asked because I saw that the builds with 2024.2 EAP are failing on main and thought this might help. I also saw that Gradle 8.5 was not working with Java 21 😄.
Given I'm the one who did the cleanup around Gradle the last time 1.5 years ago, it looks like I'm the only one who cares about Gradle. Kind of makes me maniac.
Well, no. I'm a downstream consumer, so I'm kind of involved and engaged as required.
My Gradle knowledge is rather limited, so that's why some things might look strange to you and / or why I haven't gotten around in cleaning it up. Any help is more than appreciated.
As for everything else, it looks like tests pass with a certain mockJDK versions by JetBrains and fail ridiculously with the regular way to obtain and exploit the mocks intended for the particular version of the IDE.
That's something to be contemplated about.
I think that I copied the approach for mocking the JDK from some other project. I don't remember anymore. However, I believe that now the time has come that we can easily get rid of this and just use JDK 17 (not sure if we need to mock then).
Please, don't hesitate to tell me to piss off with the changes I propose, but please do so granularly and reasonably.
I'm extremely grateful for the contributions, I always appreciate them.
If you're okay with switching from Gradle Groovy DSL to Kotlin DSL, just give me a go ahead.
I would be fine with that. I think that I tried it once, but due to my lack of Gradle and Kotlin knowledge, I didn't have much time to spend on it.
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.
Why is this needed? The formatter that we are using is https://github.com/mapstruct/mapstruct/blob/main/etc/mapstruct.xml. I'd like to keep the MapStruct format definition centralized
No description provided.