-
Notifications
You must be signed in to change notification settings - Fork 927
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
Support java modules for thrift #6076
base: main
Are you sure you want to change the base?
Conversation
if (groupIdComponents.last() == artifactIdComponents.first()) { | ||
return String.join('.', groupIdComponents + artifactIdComponents.drop(1)) | ||
} else { | ||
return String.join('.', groupIdComponents + artifactIdComponents) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When checking the specification, it seems like a identifier needs to start with a JavaLetter.
ref: docs.oracle.com/javase/specs/jls/se16/html/jls-7.html#jls-7.4
What do you think of normalizing Automatic-Module-Name by using _
instead of .
if the first character of an artifactIdComponent starts with a number?
It seems that the generated names for Scala modules violate the specification.
Manifest-Version: 1.0
Automatic-Module-Name: com.linecorp.armeria.sangria_2.12
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My initial thought was that we can't rewrite to support all of the rules defined by the jls. Having said this, assuming we won't be adding modules frequently I guess a simple rule is fine. Updated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 👍 👍
Motivation:
We received a report that thrift modules do not support java 9 modules.
When checking the specification, it seems like a identifier needs to start with a JavaLetter.
ref: https://docs.oracle.com/javase/specs/jls/se16/html/jls-7.html#jls-7.4
Unfortunately, our thrift modules contain a dot followed by a digit, which isn't a JavaLetter.
i.e.
thrift0.9
where 9 isn't a digit.I propose that for thrift modules, the last dot is omitted to avoid this issue.
i.e.
thrift0.9
->thrift09
Modifications:
automaticModuleNameOverride
so that the override configuration can live inside each moduleautomaticModuleName
is now null if not configured, or a provider if configured. This change is necessary so that the actual value ofautomaticModuleName
is not determined at configuration time, giving each module a chance to configure its ownext
property.Automatic-Module-Name
at execution time as opposed to configuration time. This is also necessary so that each module can configure its ownext
property prior to setting the name.automaticModuleNameOverride
Result: