elharo commented on code in PR #1080:
URL: https://github.com/apache/maven/pull/1080#discussion_r1158349551
##########
maven-core/src/main/java/org/apache/maven/rtinfo/internal/DefaultRuntimeInformation.java:
##########
@@ -102,7 +102,9 @@ public boolean isMavenVersion(String versionRange) {
Version current;
try {
String mavenVersion = getMavenVersion();
- Validate.validState(StringUtils.isNotEmpty(mavenVersion), "Could
not determine current Maven version");
+ if (!(mavenVersion != null && !mavenVersion.isEmpty())) {
Review Comment:
Remove double negative for clarity. That is,
```
if (mavenVersion == null || mavenVersion.isEmpty())
```
In this specific case, I'm tempted to drop the null check and just let the
resulting NullPointerException signal the problem, and I do wonder if
getMavenVersion might be guaranteed non-nullable here.
##########
maven-core/src/test/java/org/apache/maven/artifact/handler/ArtifactHandlerTest.java:
##########
@@ -40,7 +42,10 @@ public class ArtifactHandlerTest {
public void testAptConsistency() throws Exception {
File apt = getTestFile("src/site/apt/artifact-handlers.apt");
- List<String> lines = FileUtils.readLines(apt);
+ List<String> lines;
+ try (Stream<String> l = Files.lines(apt.toPath())) {
Review Comment:
Use Files.readAllLines instead, since this stream is immediately converted
into a list.
This probably needs a charset too.
##########
maven-embedder/src/test/java/org/apache/maven/cli/MavenCliTest.java:
##########
@@ -158,7 +158,7 @@ public void testCalculateDegreeOfConcurrency() {
int cpus = Runtime.getRuntime().availableProcessors();
assertEquals((int) (cpus * 2.2),
cli.calculateDegreeOfConcurrency("2.2C"));
assertEquals(1, cli.calculateDegreeOfConcurrency("0.0001C"));
- assertThrows(IllegalArgumentException.class, () ->
cli.calculateDegreeOfConcurrency("2.C"));
+ assertThrows(IllegalArgumentException.class, () ->
cli.calculateDegreeOfConcurrency("2.xC"));
Review Comment:
So this is a behavior change and a bug fix? That's good, but probably needs
a specific Jira issue focused on this particularly, and an individual PR.
Otherwise if this behavior change causes problems it's going to be quite hard
to backtrack to this change in the middle of all the other refactoring.
--
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]
