Skip to content

Add JPMS module#29

Open
mikomikotaishi wants to merge 2 commits intoMinnDevelopment:masterfrom
mikomikotaishi:master
Open

Add JPMS module#29
mikomikotaishi wants to merge 2 commits intoMinnDevelopment:masterfrom
mikomikotaishi:master

Conversation

@mikomikotaishi
Copy link
Copy Markdown

@mikomikotaishi mikomikotaishi commented Mar 6, 2026

This PR adds a JPMS module, named club.minnced.discord.jdave.

While the project could use automatic modules, considering the project is Java 25 it seems reasonable to implement proper module support for the project, and the list of packages is pretty easy to maintain. (JDA can't be modularised because it has a minimum of Java 8, but it has automatic module support.)

The reason I am hoping to add this is because some tooling fails to find the symbols, and it is better to work using module path than class path when using modular projects.

@mikomikotaishi
Copy link
Copy Markdown
Author

@MinnDevelopment Hi, do you have any thoughts about this PR?

@MinnDevelopment
Copy link
Copy Markdown
Owner

@mikomikotaishi what about the 100 compiler warnings?

module club.minnced.discord.jdave {
requires java.base;
requires transitive net.dv8tion.jda;
requires org.jspecify;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Annotations should not be a required dependency to use this library. I would probably use requires static transitive here

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Ah, good catch, I've also applied requires static for org.jspecify as well because it's for annotations.

@MinnDevelopment
Copy link
Copy Markdown
Owner

The README has a section about native access warnings, which need to include a section on how to properly allow access when this is used as a module. Currently, it only specifies it for unnamed modules.

Currently, there is no real tests if the library can be loaded properly when modules are used, since the JUNIT tests all use unnamed modules. I assume that it won't work since the library is loaded via Class#getResourceAsStream.

@mikomikotaishi
Copy link
Copy Markdown
Author

I think the following changes should fix the warnings, which are mostly tied to restricted methods from FFM, @SuppressWarnings("restricted") should solve it

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