[
https://issues.apache.org/jira/browse/MNG-6825?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17708811#comment-17708811
]
ASF GitHub Bot commented on MNG-6825:
-------------------------------------
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.
> Cleaning Up dependencies
> plexus-utils/maven-shared-utils/commons-lang3/commons-io
> ---------------------------------------------------------------------------------
>
> Key: MNG-6825
> URL: https://issues.apache.org/jira/browse/MNG-6825
> Project: Maven
> Issue Type: Improvement
> Components: Bootstrap & Build
> Affects Versions: 3.6.3
> Reporter: Karl Heinz Marbaise
> Assignee: Karl Heinz Marbaise
> Priority: Minor
> Fix For: 4.0.x-candidate
>
>
> Experiment to see if we can get rid of commons-lang3 dependency.
> * Identified code duplication between:
> ** {{org.apache.commons.lang3.StringUtils}} from {{commons-lang3}} (501879
> Bytes commons-lang3-3.8.1.jar)
> ** {{org.codehaus.plexus.util.StringUtil}} from {{plexus-utils}} (261801
> Bytes plexus-utils-3.2.1.jar)
> ** {{org.apache.maven.shared.utils}} from {{maven-shared-utils}} (166562
> Bytes maven-shared-utils-3.2.1.jar)
> ** We should replace the self implemented with
> {{org.codehaus.plexus.util.StringUtil}}. or with {{maven-shared-utils}}
> * {{commons-io}} usage by {{maven-shared-utils}} (208700 Bytes
> commons-io-2.5.jar)
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
