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 a config variable to allow users to set the Java executable location (fixes issue #57) #273

Merged
merged 2 commits into from
May 29, 2024

Conversation

parkerlamb
Copy link
Contributor

Fixes issue #57.

This adds a new configuration variable called javaHomeOverride. This allows users to explicitly set JAVA_HOME if it is not present or if they wish to use a different Java installation. The Java executable is looked up at ${javaHomeOverride}/bin/java in this case.

This also changes the default behavior to use $JAVA_HOME/bin/java as the Java executable to be consistent.

If neither of javaHomeOverride or the JAVA_HOME environment variable are set then the extension attempts to launch java in PATH as before.

I'm happy to adjust this as needed, feedback welcome.

Copy link
Owner

@mike-lischke mike-lischke left a comment

Choose a reason for hiding this comment

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

Please add a note to the documentation (doc/extension-settings.md).

@mike-lischke
Copy link
Owner

You also have to complete the DCO check. Follow the "Details" link to learn how to do that.

…ws users to explicitly set JAVA_HOME if it is not present or if they wish to use a different Java installation. The Java executable is looked up at `${javaHomeOverride}/bin/java` in this case.

This also changes the default behavior to use `$JAVA_HOME/bin/java` as the Java executable.

If neither of `javaHomeOverride` or the JAVA_HOME environment variable are set then the extension attempts to launch `java` in PATH as before.

Fixes issue mike-lischke#57.

Signed-off-by: Parker Lamb <parkerlamb@users.noreply.github.com>
Signed-off-by: Parker Lamb <parkerlamb@users.noreply.github.com>
@parkerlamb
Copy link
Contributor Author

Thanks for the comments - I think I've addressed everything

@mike-lischke mike-lischke merged commit 6dae3ef into mike-lischke:master May 29, 2024
3 checks passed
@mike-lischke
Copy link
Owner

Thanks!

@7c00
Copy link

7c00 commented Aug 21, 2024

hi @mike-lischke, could you publish a new release to include this feature?

@RedHotUnicorn
Copy link

hi @mike-lischke , we really would like to get an ew release with that feature. thanks

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.

4 participants