move VersionMappings stuff to a separate module and make it public #13
Conversation
Generate changelog in
|
| @@ -17,7 +17,7 @@ | |||
|
|
|||
| import org.apache.maven.artifact.versioning.ComparableVersion; | |||
|
|
|||
| final class VersionMapping { | |||
| public final class VersionMapping { | |||
There was a problem hiding this comment.
I suspect we'll need to make the getters public as well
| @@ -17,7 +17,7 @@ | |||
|
|
|||
| import org.apache.maven.artifact.versioning.ComparableVersion; | |||
There was a problem hiding this comment.
I think this is an implementation dependency, so it can't be exposed with a public getter. Can we abstract it away somehow such that we're not bound to the maven ComparableVersion type?
There was a problem hiding this comment.
so one alternative to this is that we could leave VersionMapping package-private, and instead change the public VersionMappings#getReplacement to return an Optional<MavenCoordinate> instead of Optional<String>. From the perspective of something outside of this module, a caller doesn't care so much with inspecting the individual parts of VersionMapping and likely only cares about asking "what is the replacement that will be used for this dependency?"
There was a problem hiding this comment.
Sounds good to me, reducing the exposed surface is always a plus
This reverts commit 3b5568a.
…-package-alignment into blaub/split-packages
| ComparableVersion getMaxJakartaVersionWithJavaxNamespace() { | ||
| return maxJakartaVersionWithJavaxNamespace; | ||
| } |
There was a problem hiding this comment.
Thoughts on removing this method entirely in favor of building ComparableVersion on an as-needed basis in VerssionMappings?
There was a problem hiding this comment.
one downside is that it becomes less clear that this mapping could technically apply to a range of versions, but i think in practice there's only one Jakarta EE release with the old javax namespace, so it's really always a single version.
| public static Map<String, VersionMapping> getMappings() { | ||
| return mappings; | ||
| } |
There was a problem hiding this comment.
Perhaps this should return Collection<VersionMapping> since the key is an implementation detail for quick lookups? we could return mappings.values();
also make VersionMappings#getMappings return Collection<VersionMapping>
|
Released 0.2.0 |
No description provided.