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

Made Java installation as connection features #2283

Merged
merged 4 commits into from
Oct 9, 2024

Conversation

sebjulliand
Copy link
Collaborator

Changes

This PR takes the Java discovery part of #2195.
The different possible Java installation locations are made into connection features.

These features are now used to get Java Home path when configuring the debug service.

How to test this PR

  1. Run the Debug test case
  2. Generate a Debug Service certificate
  3. Check Java Home in the Debug Service configuration file

Checklist

  • have tested my change

@sebjulliand sebjulliand added the enhancement New feature or request label Oct 7, 2024
@sebjulliand sebjulliand self-assigned this Oct 7, 2024
@worksofliam worksofliam self-assigned this Oct 7, 2024
@worksofliam
Copy link
Contributor

Beautiful code change. Will get it tested ASAP.

src/api/IBMi.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@worksofliam worksofliam left a comment

Choose a reason for hiding this comment

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

I think my only concern is the finding of JDK version without reloading. Other than that, very nice change!

@sebjulliand
Copy link
Collaborator Author

I think my only concern is the finding of JDK version without reloading. Other than that, very nice change!

@worksofliam Can you try again? I connected to an existing system created and connected before the PR, and the reloading occurred as expected.

The existing test was accurate:
image

Or am I missing something obvious? 😅

Copy link
Contributor

@worksofliam worksofliam left a comment

Choose a reason for hiding this comment

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

@sebjulliand Tested again and it worked as expected. I connected to an existing system and it grabbed the JDK correctly. Thanks!

Merge away!

@sebjulliand sebjulliand merged commit 7fefd51 into master Oct 9, 2024
1 check passed
@sebjulliand sebjulliand deleted the feature/discoverjdk branch October 9, 2024 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants