Skip to content

Upgrade minimum Java version from 17 to 21#10

Open
dillonvargo wants to merge 1 commit into
masterfrom
devin/1769705544-java-21-minimum
Open

Upgrade minimum Java version from 17 to 21#10
dillonvargo wants to merge 1 commit into
masterfrom
devin/1769705544-java-21-minimum

Conversation

@dillonvargo

Copy link
Copy Markdown
Owner

Description of what I changed

This PR upgrades the minimum required Java version from 17 to 21 across the codebase:

  • pom.xml: Removed the Java 17 Maven profile since it's no longer needed. The Java 21 and Java 24 profiles remain for version-specific dependency management.
  • OpenmrsUtil.java: Rewrote validateJavaVersion() to properly check for Java 21 minimum. The previous implementation only rejected Java 7 and earlier using an outdated regex pattern. The new implementation parses the major version from both old-style (1.x.y) and new-style (21.0.1) version strings.
  • README.md: Updated documentation to reflect Java 21 as the minimum required version.
  • startup.sh: Simplified the debug port configuration by removing the Java 8 conditional branch, since Java 21+ always requires the *: prefix for the debug address.

The Dockerfile was verified and already correctly uses Java 21.

Issue I worked on

see https://issues.openmrs.org/browse/TRUNK-

Checklist: I completed these to help reviewers :)

  • My IDE is configured to follow the code style of this project.
  • I have added tests to cover my changes. (If you refactored existing code that was well tested you do not have to add tests)
  • I ran mvn clean package right before creating this pull request and added all formatting changes to my commit.
  • All new and existing tests passed.
  • My pull request is based on the latest changes of the master branch.

Items for reviewer attention

  1. Version parsing logic in OpenmrsUtil.java - Please verify the parsing handles all expected Java version string formats correctly (e.g., "21.0.1", "21", "21-ea", "1.8.0_xxx")
  2. No unit tests added - The validateJavaVersion() method change could benefit from test coverage for the new parsing logic

Requested by: Dillon Vargo (@dillonvargo)
Link to Devin run: https://app.devin.ai/sessions/720a2eb76f6c41c1b2e02d66b76475e5

Co-Authored-By: Dillon Vargo <dillonvargo@gmail.com>
@devin-ai-integration

Copy link
Copy Markdown

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

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