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

Introduce full module-info.java #127

Closed
xenoterracide opened this issue Sep 14, 2024 · 5 comments
Closed

Introduce full module-info.java #127

xenoterracide opened this issue Sep 14, 2024 · 5 comments
Assignees
Labels
in: infrastructure Infrastructure related topics type: enhancement New feature or request
Milestone

Comments

@xenoterracide
Copy link

xenoterracide commented Sep 14, 2024

#47 is great, but I think that these jars are ideal candidates to have a full module-info.java perhaps using the java9 (like jspecify does) directory if java 8 compatibility is required. This actually causes a real problem for future compatibility here. Since the DDD interfaces are part of your API a consumer should be using requires transient but JPMS warns if you do this for an automatic-module.

I'd be willing to put in some legwork here.

@odrotbohm odrotbohm changed the title JPMS: migrate to full module-info Introduce full module-info.java Sep 20, 2024
@odrotbohm odrotbohm added type: enhancement New feature or request in: infrastructure Infrastructure related topics labels Sep 20, 2024
odrotbohm added a commit that referenced this issue Sep 20, 2024
odrotbohm added a commit that referenced this issue Sep 20, 2024
@odrotbohm
Copy link
Member

I've put the module descriptors in place. Would you mind building the current snapshots locally and giving the results a try?

@odrotbohm odrotbohm added this to the 1.10 milestone Sep 20, 2024
@odrotbohm odrotbohm self-assigned this Sep 20, 2024
@xenoterracide
Copy link
Author

I'm just looking at the commit. You're going to want to put opens for these packages as well. Otherwise libraries won't be able to read them reflectively.

compilation is fine, but I have no idea what the impact of having an automatic-module in the same jar as one with a module-info is. I don't think you need to strip it out for kotlin, but I haven't tested.

You should be able to add comprehensive tests by adding a module-info to your test suites. That will also require you to rename the test suite package.

odrotbohm added a commit that referenced this issue Sep 25, 2024
Remove automatic module names from manifests. Additionally open packages for reflection.
@odrotbohm
Copy link
Member

That's great feedback, thanks! I've removed the automatic module names and added the opens clauses to the module descriptors. Considering the invasiveness of JPMS testing, I don't think we're going to introduce dedicated integration tests just for the metadata, as there's no corner cases to test for. All code is open and accessible to anyone.

@xenoterracide
Copy link
Author

xenoterracide commented Sep 25, 2024

You don't have to do what I'm about to say, I'm not certain I recommend it. I prefer explicitness. repeat this is just an FYI.

You could write this instead. I forgot it existed because none of the modules I've been developing are pure API and so I prefer to be explicit as I don't need things like spring iterating over util packages. It effectively does the same thing. YMMV, just thought I'd mention it since I saw it on javas docs when I was looking for something else.

open module modulename {
   // module directives
}

note: going to give the latest snapshot a test, either later today, or perhaps tomorrow.

@odrotbohm
Copy link
Member

Updated, thanks! 🙇

@odrotbohm odrotbohm modified the milestones: 1.10.0, 2.0 RC1 Nov 20, 2024
odrotbohm added a commit that referenced this issue Nov 20, 2024
Remove automatic module names from manifests. Additionally open packages for reflection.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: infrastructure Infrastructure related topics type: enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants