garydgregory commented on PR #1369:
URL: https://github.com/apache/commons-lang/pull/1369#issuecomment-2781402240
-1 as the PR stands: This new code has zero unit tests, duplicates too much
existing code, and doesn't even compile:
```
[INFO] -------------------------------------------------------------
[WARNING] COMPILATION WARNING :
[INFO] -------------------------------------------------------------
[WARNING]
/Users/garygregory/git/commons-lang/src/test/java/org/apache/commons/lang3/builder/ReflectionDiffBuilderTest.java:[172,52]
non-varargs call of varargs method with inexact argument type for last
parameter;
cast to java.lang.String for a varargs call
cast to java.lang.String[] for a non-varargs call and to suppress this
warning
[INFO] 1 warning
[INFO] -------------------------------------------------------------
[INFO] -------------------------------------------------------------
[ERROR] COMPILATION ERROR :
[INFO] -------------------------------------------------------------
[ERROR]
/Users/garygregory/git/commons-lang/src/test/java/org/apache/commons/lang3/StringUtilsTest.java:[2452,31]
reference to split is ambiguous
both method split(java.lang.String,java.lang.String) in
org.apache.commons.lang3.StringUtils and method
split(java.util.List<java.lang.String>,java.lang.String) in
org.apache.commons.lang3.StringUtils match
[INFO] 1 error
[INFO] -------------------------------------------------------------
[INFO]
------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO]
------------------------------------------------------------------------
[INFO] Total time: 6.812 s
[INFO] Finished at: 2025-04-06T08:34:02-04:00
[INFO]
------------------------------------------------------------------------
[ERROR] Failed to execute goal
org.apache.maven.plugins:maven-compiler-plugin:3.13.0:testCompile
(default-testCompile) on project commons-lang3: Compilation failure
[ERROR]
/Users/garygregory/git/commons-lang/src/test/java/org/apache/commons/lang3/StringUtilsTest.java:[2452,31]
reference to split is ambiguous
[ERROR] both method split(java.lang.String,java.lang.String) in
org.apache.commons.lang3.StringUtils and method
split(java.util.List<java.lang.String>,java.lang.String) in
org.apache.commons.lang3.StringUtils match
[ERROR]
```
You must not have run 'mvn' by itself to fix all the build issues as
documented in this PR's template!
IMO, this isn't generally useful, and I'm curious what others think. We
already have `StringUtils.split(String, String)` and this PR duplicates most of
that code.
A low-level utility method in `StringUtils` is more useful with a String
input, like `StringUtils.split(String, String)` which begs the question of why
the PR is not reusing it. Aside from that, I don't think we need to open the
door for List-to-List methods for this one case; it seems like it should be
left to the application.
Detail: The new worker method is called only once with hardcoded parameters,
so the hard-coded parameters should be removed. This feels like unfinished
copy-pasta 😞
--
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: issues-unsubscr...@commons.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
