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

Fixed JPMS declarations. #11831

Merged

Conversation

sbordet
Copy link
Contributor

@sbordet sbordet commented May 22, 2024

Some require transitive was missing, and has been added.

In other cases, making a field private (e.g. for log instances) made the warning go away.

In another case, removed explicit dependency on websocket core exception, as it was not necessary.

Clean up the POMs for jetty-eeN-annotations, that had unnecessary dependencies.

Some require transitive was missing, and has been added.
In other cases, making a field private (e.g. for log instances) made the warning go away.
In another case, removed explicit dependency on websocket core exception, as it was not necessary.

Clean up the POMs for jetty-eeN-annotations, that had unnecessary dependencies.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
@sbordet sbordet requested review from gregw, janbartel and lorban May 22, 2024 10:18
@sbordet sbordet linked an issue May 22, 2024 that may be closed by this pull request
Copy link
Contributor

@lorban lorban left a comment

Choose a reason for hiding this comment

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

Looks good to me, but how did you notice all those transitive keywords were missing? And is that testable?

@sbordet
Copy link
Contributor Author

sbordet commented May 22, 2024

how did you notice all those transitive keywords were missing? And is that testable?

Compiling the project (either Maven or IntelliJ) reports the warnings.
Fixed them manually one by one.

I don't think it's testable, unless we want to be very strict and tell the compiler to fail on warnings, but I guess that would fail on a million other warnings.

@olamy
Copy link
Member

olamy commented May 22, 2024

Copy link
Contributor

@gregw gregw left a comment

Choose a reason for hiding this comment

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

JPMS: so much work for so little gain!

@sbordet sbordet merged commit 36cdc12 into jetty-12.0.x May 23, 2024
10 checks passed
@sbordet sbordet deleted the fix/jetty-12.0.x/11805/jpms-requires-transitive branch May 23, 2024 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

module-info.java warnings for ManagedObject, ManagedAttribute, etc
4 participants