Repository navigation
feat(build)!: ship an explicit JPMS module and vendor the country code enum - #474
Merged
Merged
Conversation
nv-i18n has no module descriptor and no Automatic-Module-Name, so its module name is derived from its file name and cannot be required safely from an explicit module.
The jar now carries a real module-info instead of relying on the Automatic-Module-Name fallback, so consumers get exports, transitive requires and the reflective access Gson needs declared rather than inferred.
An explicit module makes javadoc emit a module-scoped tree, which moves every class page. GitHub Pages has no server-side redirects, so a root 404.html maps the old paths onto the new ones.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## beta #474 +/- ##
============================================
+ Coverage 29.66% 31.65% +1.98%
- Complexity 679 683 +4
============================================
Files 323 324 +1
Lines 8851 9109 +258
Branches 776 777 +1
============================================
+ Hits 2626 2883 +257
Misses 5669 5669
- Partials 556 557 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The jar becomes an explicit JPMS module. Until now it shipped as an automatic module named by the
Automatic-Module-Namemanifest entry, which gives consumers a stable name but nothing else: no exports, no declared dependencies, and no way forjlinkto treat the library as a real module.module-info.javanow declares all 38 packages as exports, marks Gson and both HttpComponents modulesrequires transitivebecause their types appear in this library's own public signatures, and opens the five model packages Gson populates reflectively. The module name is unchanged, so an existingrequires se.michaelthelin.spotify;keeps working, and nothing changes for code on the classpath.That forced the
com.neovisionaries:nv-i18ndependency out.CountryCodeis part of the public API, so a module would have torequires transitiveit, but nv-i18n has neither a module descriptor nor anAutomatic-Module-Name, so its name is derived from its file name. Publishing a descriptor that depends on a derived name breaks every consumer if the artifact is ever renamed or later gains a real one, and it keeps consumers from producing a cleanjlinkimage. The maintained fork proposed in #467 has the same gap, so switching forks would not have helped.The library only ever needed the alpha-2 string from it: 35
market(CountryCode)parameters, twocountryparameters, oneCountryCode[]field, and oneLanguageCodelookup inside anassert.se.michaelthelin.spotify.enums.CountryCodeis generated fromLocale.getISOCountries()and keeps the constant names, so callers change one import and nothing else. It carriesXKon top of the JDK's list: checked against Spotify's own 186-market response and everyavailable_marketsarray in the fixtures, that is the only code Spotify uses that the JDK omits. The oneLanguageCodeuse was an assertion, and is now aLocale.getISOLanguages()check.ModelObjectGsonregisters an adapter for the new enum alongside the others, which also makes it case-insensitive and nulls an unknown code rather than the object around it. Nothing coveredCountryCode[]before, so there is a test for it; removing the registration fails it.An explicit module also makes javadoc emit a module-scoped tree, moving every class page under a
se.michaelthelin.spotify/path segment. Since GitHub Pages has no server-side redirects, a root404.htmlmaps the old paths onto the new ones, which is one file rather than a stub per page. It takes effect when beta merges to main, at which point both the move and the redirect land together.Verified against a real consumer rather than only in isolation: a downstream JPMS application builds and
jlinks against this, and the resulting runtime image listsse.michaelthelin.spotifyas a named module instead of folding it into a merged one.Two things worth knowing that this PR does not change.
Automatic-Module-Namestays in the manifest even though the descriptor supersedes it, because it costs one line and still covers tooling that reads the manifest but notmodule-info. Andmvn packagecompilesexamples/into the module while-P releasedoes not, so CI validates a module that is not quite the one that gets published; the published artifact is correct, but amodule-info.javareferencing an example package would pass CI and fail at release.BREAKING CHANGE:
marketandcountryparameters takese.michaelthelin.spotify.enums.CountryCodeinstead ofcom.neovisionaries.i18n.CountryCode. The constant names are identical, so this is an import change.