Rename versionRange attribute to compatibilityRange #968
Conversation
htztomic
force-pushed
the
gh-418
branch
2 times, most recently
from
305ed5d
to
c23860a
Compare
There was a problem hiding this comment.
Thanks for the PR. LGTM in general with the following suggestions:
- I don't think the low-level should be renamed. It represents a
VersionRangeso the current name sounds fine to me - I am not keen to introduce a v2.2 of the metadata at this point.
Let me know what you think, thanks!
| @@ -34,7 +34,7 @@ | |||
| * | |||
| * @author Stephane Nicoll | |||
| */ | |||
| public class VersionRange { | |||
| public class CompatibilityRange { | |||
There was a problem hiding this comment.
I don't think we should rename the low-level object type. It does represent a version range and the purpose of this task is to requalify a particular use of it.
There was a problem hiding this comment.
Agreed upon keeping VersionRange as the underlying name as it does represent a range of the low level object Version.
There was a problem hiding this comment.
FTR, there is actually right now another use of VersionRange, see OnGradleVersionCondition.
| * | ||
| * @author HaiTao Zhang | ||
| */ | ||
| public class InitializrMetadataV22JsonMapper extends InitializrMetadataV21JsonMapper { |
There was a problem hiding this comment.
I don't think we are ready to introduce 2.2 yet. This change should be part of V3
There was a problem hiding this comment.
Agreed upon grouping multiple changes to V3 instead of having a V2.2 of the InitializrMetadataJsonMapper.
pom.xml
Outdated
| @@ -361,9 +361,9 @@ | |||
| <artifactId> | |||
| maven-checkstyle-plugin | |||
| </artifactId> | |||
| <versionRange> | |||
| <compatibilityRange> | |||
There was a problem hiding this comment.
This was a too agressive search and replace here.
snicoll
added
the
status: waiting-for-feedback
htztomic
force-pushed
the
gh-418
branch
2 times, most recently
from
70fc278
to
1384ae2
Compare
snicoll
added
type: enhancement
and removed
status: waiting-for-feedback
|
@htztomic well done, thank you very much! |
This is a fix to GitHub issue #418. The versionRange attribute has been renamed to compatibilityRange.