Skip to content

feat: implemented support for Jdk distro and tags #18

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

quintesse
Copy link
Contributor

No description provided.

@quintesse quintesse force-pushed the jdk_tags branch 2 times, most recently from 023698f to 4b0127b Compare March 13, 2025 18:14
@quintesse quintesse marked this pull request as draft March 14, 2025 00:40
@quintesse quintesse requested a review from Copilot March 27, 2025 10:23
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements support for JDK distributions and tags by updating method signatures, JDK creation logic, and associated tests across multiple modules. Key changes include:

  • Updating provider and installer APIs to accept distro and tag parameters.
  • Enhancing the JDK model to include distro and tags.
  • Adding and adapting tests to validate filtering and distribution listing.

Reviewed Changes

Copilot reviewed 19 out of 20 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/test/java/dev/jbang/devkitman/jdkproviders/MockJdkProvider.java Updated listAvailable() to accept distro and tag parameters.
src/test/java/dev/jbang/devkitman/jdkproviders/JdkProviderWrapper.java Adapted method signatures to forward distro and tag parameters.
src/test/java/dev/jbang/devkitman/TestJdkInstaller.java Added tests to validate distro and tag filtering.
src/main/java/dev/jbang/devkitman/util/JavaUtils.java Added readTagsFromReleaseFile() utility to extract tags from the release file.
src/main/java/dev/jbang/devkitman/util/FileUtils.java Refactored deletePath method to delegate deletion to a quiet variant.
src/main/java/dev/jbang/devkitman/jdkproviders/PathJdkProvider.java Updated JDK creation to include distro and tags.
src/main/java/dev/jbang/devkitman/jdkproviders/LinkedJdkProvider.java Updated JDK creation with distro and tag information.
src/main/java/dev/jbang/devkitman/jdkproviders/JavaHomeJdkProvider.java Updated JDK creation with distro and tag information.
src/main/java/dev/jbang/devkitman/jdkproviders/JBangJdkProvider.java Adjusted method signatures to pass distro and tags.
src/main/java/dev/jbang/devkitman/jdkproviders/DefaultJdkProvider.java Updated JDK creation to include distro and tags.
src/main/java/dev/jbang/devkitman/jdkproviders/CurrentJdkProvider.java Updated JDK creation to include distro and tags.
src/main/java/dev/jbang/devkitman/jdkproviders/BaseFoldersJdkProvider.java Updated JDK creation to include distro and tags.
src/main/java/dev/jbang/devkitman/jdkinstallers/FoojayJdkInstaller.java Refactored installer logic to support distro and tag parameters in URL generation and JDK creation.
src/main/java/dev/jbang/devkitman/JdkProvider.java Updated interface methods to require distro and tags for JDK creation and listing.
src/main/java/dev/jbang/devkitman/JdkManager.java Extended listing and default JDK setting to account for distro and tag information.
src/main/java/dev/jbang/devkitman/JdkInstaller.java Updated installer interface methods to accept distro and tag parameters.
src/main/java/dev/jbang/devkitman/Jdk.java Enhanced JDK model to include distro and tags.
src/main/java/dev/jbang/devkitman/Distro.java Introduced a new model for representing JDK distributions.
Files not reviewed (1)
  • src/test/resources/testDistros.json: Language not supported
Comments suppressed due to low confidence (2)

src/main/java/dev/jbang/devkitman/JdkProvider.java:185

  • Calling listAvailable(null, null) may lead to unexpected behavior if implementations do not properly handle null values for distros and tags. Consider defaulting to an empty string for distros and an empty set for tags to ensure consistency.
return JdkManager.getJdkBy(listAvailable(null, null).stream(), Jdk.Predicates.id(idOrToken))

src/main/java/dev/jbang/devkitman/jdkinstallers/FoojayJdkInstaller.java:256

  • The logic for setting 'release_status' depends strictly on the presence of 'ea' or 'ga' tags and defaults to 'ga,ea' when neither is present. Verify that this behavior fully aligns with the intended filtering of releases, and consider adding inline documentation or validation for the expected tag values.
if (tags.contains("ea")) { params.put("release_status", "ea"); } else if (tags.contains("ga")) { params.put("release_status", "ga"); } else { params.put("release_status", "ga,ea"); }

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.

1 participant