gnodet-bot commented on code in PR #11770:
URL: https://github.com/apache/maven/pull/11770#discussion_r4306808415
##########
maven-core/src/main/java/org/apache/maven/toolchain/RequirementMatcherFactory.java:
##########
@@ -79,6 +81,32 @@ public boolean matches(String requirement) {
}
}
+ private static final Pattern PATTERN_MAJOR_VERSION =
Pattern.compile("^[0-9]+$");
+ private static final Pattern PATTERN_MAJOR_MINOR_VERSION =
Pattern.compile("^[0-9]+\\.[0-9]+$");
+
+ private VersionRange convertRequirementToVersionRange(String
requirement)
Review Comment:
💡 **Nit:** `convertRequirementToVersionRange` accesses no instance state —
only its `requirement` parameter, the two `private static final` Pattern
constants, and the static method `VersionRange.createFromVersionSpec`. It
should be `private static` for correctness and consistency with the static
fields declared immediately above it.
```suggestion
private static VersionRange convertRequirementToVersionRange(String
requirement)
```
##########
maven-core/src/test/java/org/apache/maven/toolchain/RequirementMatcherFactoryTest.java:
##########
@@ -63,4 +70,29 @@ public void testCreateVersionMatcher() {
matcher = RequirementMatcherFactory.createVersionMatcher("1.5");
assertEquals("1.5", matcher.toString());
Review Comment:
💡 **Nit:** The `"1.5"` provider is only tested for `toString()`. The new
prefix-matching behavior is exercised for `"1.5.2"` and `"11.55.22"` providers,
but not for a two-part version as a provider. Consider adding:
```suggestion
matcher = RequirementMatcherFactory.createVersionMatcher("1.5");
assertTrue(matcher.matches("1")); // Major matches
assertTrue(matcher.matches("1.5")); // Self-match
assertFalse(matcher.matches("2")); // Wrong major
assertFalse(matcher.matches("1.6")); // Wrong minor
assertEquals("1.5", matcher.toString());
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
