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

✨jdk tools version management tool, switch from jenv to sdkman #112

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

tonycody
Copy link

@tonycody tonycody commented Nov 25, 2022

approval
#110

@tonycody tonycody changed the title ✨jdk tools version management tool, switch from jenv to sdkman ✨jdk tools version management tool, switch from jenv to sdkman # 110 Nov 25, 2022
@tonycody tonycody changed the title ✨jdk tools version management tool, switch from jenv to sdkman # 110 ✨jdk tools version management tool, switch from jenv to sdkman #110 Nov 25, 2022
@tonycody tonycody changed the title ✨jdk tools version management tool, switch from jenv to sdkman #110 ✨jdk tools version management tool, switch from jenv to sdkman Nov 25, 2022
@cstamas
Copy link
Member

cstamas commented Nov 25, 2022

-1 for several reasons:

  • Nor jenv nor sdkman is "must" for build, this PR makes it looks like it is.
  • use of wrapper is encouraged, while this PR would force mvn install with forced sdkman
  • if you look at (parent) POM, you will see enforcer rule for maven version. I'd rather extend that with enforcer rule for Java version as well, instead to force users to one given Java provider (I personally use Temurin, but what if someone else would use Graal or whatever?)

@tonycody
Copy link
Author

@cstamas Just historically using jenv, sdkman is just a little more friendly than jenv. maven specified by sdkman does not affect the maven wrapper, Just historically using Jenv, SDkman is just a little more friendly than Jenv. You can remove maven's qualification!

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

Successfully merging this pull request may close these issues.

2 participants