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

Add Configuration for Kotlin's All-Open Plugin for JPA Entities #1576

Closed

Conversation

YangSiJun528
Copy link
Contributor

@YangSiJun528 YangSiJun528 commented Oct 9, 2024

Currently, the KotlinJpaGradleBuildCustomizer class is responsible for configuring either javax or jakarta dependencies. I believe a cleaner approach would be to split this functionality into two separate classes:

  • KotlinJavaxJpaGradleBuildCustomizer
  • KotlinJakartaJpaGradleBuildCustomizer

I'm unsure whether these new classes should inherit from KotlinJpaGradleBuildCustomizer or be designed as independent classes that implement BuildCustomizer. (similar to KotlinJpaMavenBuildCustomizer)

I would appreciate feedback on this approach.

The strikethrough was used because JavaEE support is no longer needed. This makes the previous concerns irrelevant.

Fixes gh-1572

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Oct 9, 2024
@mhalbritter mhalbritter self-assigned this Oct 9, 2024
@YangSiJun528
Copy link
Contributor Author

YangSiJun528 commented Oct 10, 2024

I apologize for not thoroughly checking the code before opening this PR.
I'm currently addressing several bugs, so no need to review it just yet.

I'll tag you once everything is resolved. Feel free to close the PR, and I'll open a new one after the fixes.

Copy link
Contributor

@mhalbritter mhalbritter left a comment

Choose a reason for hiding this comment

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

Hey @YangSiJun528,

thanks for the PR. I've left some comments.

if (this.buildMetadataResolver.hasGroupId(build, "jakarta.persistence")) {
customizeAllOpenWithJakarta(build);
}
else if (this.buildMetadataResolver.hasGroupId(build, "javax.persistence")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't need to support JavaEE anymore. Our baseline is Spring Boot 3.2, which already uses JakartaEE.

if (this.buildMetadataResolver.hasGroupId(build, "jakarta.persistence")) {
customizeAllOpenWithJakarta(configuration);
}
else if (this.buildMetadataResolver.hasGroupId(build, "javax.persistence")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't need to support JavaEE anymore.

}

@Test
void customizeWhenJavaxPersistencePresentShouldCustomizeAllOpenWithJavax() {
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't need to support JavaEE anymore.

@@ -31,12 +31,6 @@

import static org.assertj.core.api.Assertions.assertThat;

/**
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you remove this?

}

@Test
void customizeWhenJavaxPersistencePresentShouldCustomizeAllOpenWithJavax() {
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to support JavaEE anymore.

@mhalbritter mhalbritter added the status: waiting-for-feedback We need additional information before we can continue label Oct 10, 2024
@YangSiJun528
Copy link
Contributor Author

I have fixed the bug and incorporated your feedback, @mhalbritter :

  • Restored the accidentally deleted comments.
  • Removed the support for JavaEE.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Oct 10, 2024
@mhalbritter mhalbritter added type: enhancement and removed status: feedback-provided Feedback has been provided status: waiting-for-triage An issue we've not yet triaged labels Oct 11, 2024
@mhalbritter
Copy link
Contributor

Thanks @YangSiJun528 !

@mhalbritter mhalbritter added this to the 0.22.0 milestone Oct 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Configure Kotlin's all-open plugin for JPA entities
3 participants