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

Group maven/gradle wrappers as 'build-launcher' and add dependencies #8694

Merged
merged 1 commit into from
Jan 20, 2024

Conversation

gzsombor
Copy link
Member

No description provided.

Copy link

codecov bot commented Jan 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (e28df1e) 100.00% compared to head (220f65a) 100.00%.

Additional details and impacted files
@@             Coverage Diff             @@
##                main     #8694   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
  Complexity      2821      2821           
===========================================
  Files            719       719           
  Lines          12404     12405    +1     
  Branches         250       250           
===========================================
+ Hits           12404     12405    +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -15,6 +15,7 @@ public enum JHLiteFeatureSlug implements JHipsterFeatureSlugFactory {
DUMMY_SCHEMA("dummy-schema"),
FRONT_BROWSER_TEST("front-browser-test"),
JAVA_BUILD_TOOL("java-build-tool"),
JAVA_BUILD_LAUNCHER("java-build-launcher"),
Copy link
Contributor

Choose a reason for hiding this comment

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

How about "java-build-tool-wrapper"?
I'm not sure that "build-launcher" is meaningful.

Copy link
Member Author

Choose a reason for hiding this comment

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

That could work as well 😄

@murdos murdos enabled auto-merge January 20, 2024 21:48
@murdos murdos merged commit e121df8 into jhipster:main Jan 20, 2024
35 checks passed
@pascalgrimaud
Copy link
Member

Not sure about this change, as maven-java already added the maven-wrapper

I add this module as standalone for 2 reasons:

  • to add this wrapper on existing projects which were not generated with JHLite
  • to sometimes upgrade wrapper

So I think it should stay as standalone and not depend on other module

@gzsombor
Copy link
Member Author

Yes, I've just noticed, that the maven wrapper is still installed. I would rather remove it from when MAVEN_JAVA is applied. Currently I'm trying to create a more modern project setup, with a declarative version and environment manager like mise/rtx/asdf, so these wrapper executables are very misleading in my project setup.

@pascalgrimaud
Copy link
Member

Yes it could be possible to remove it from the maven_java, but in this case, it should be rename to maven_pom_xml_java

I would suggest to open a new ticket to discuss about what we should do for this maven-wrapper, because with this change, this module becomes useless :-p

@pascalgrimaud
Copy link
Member

And renaming module is a small breaking change for existing projects

@murdos
Copy link
Contributor

murdos commented Jan 21, 2024

Yes it could be possible to remove it from the maven_java, but in this case, it should be rename to maven_pom_xml_java

Sorry, I did not notice this issue before merging the PR...

I would also remove the maven wrapper from the maven_java module, I often encounter projects that use maven but don't need the maven wrapper. BTW I just verified: the gradle module also generates the wrapper, but I think I just did that to mimic the maven behavior. I was persuaded that gradle wrapper was an opt-in option by using the gradle-wrapper module.

However I don't think we should rename "maven_java", event if we remove the wrapper: even if there's just a pom.xml generated it's a building block of all other modules, and marks the choice of the java build tool.
I don't think you would want to rename the gradle_java module to "gradle_build.gradle_settings_gradle_libs_versions_java" (corresponding to the 3 generated files for gradle: build.gradle.kts, settings.gradle.kts and gradle/libs.versions.toml)

@gzsombor
Copy link
Member Author

I've opened a ticket about this #8697 for further discussions

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.

None yet

3 participants