garydgregory commented on PR #1768: URL: https://github.com/apache/commons-lang/pull/1768#issuecomment-5181169077
@jlerbsc Closing this PR. There are too many issues with it to justify further review. The PR description is misleading. For example: > new Integer(i) / new Long(l) / new Boolean(b) / new Double(d) -> Integer.valueOf(i) / ... There are no such changes in this PR. Likewise: > new String(s) -> s There are no such changes either. At the time of writing, across both commits, neither of these claimed transformations exists. Why does this PR contain two commits instead of one? Why include a merge commit rather than submitting a clean branch? The PR also states: > mvn clean verify passes locally. This is not what our PR template asks contributors to do. We ask authors to run the default Maven build (mvn), which executes all required checks. The template was clearly not followed. The only actual code change in this PR is replacing calls to `new String(char[])` with `String.valueOf(char[])`. Since `String.valueOf(char[])` simply delegates to `new String(char[])`, this change adds an unnecessary layer of indirection with no functional, performance, or readability benefit. It therefore makes the code less direct without providing any value. Consequently, the "Why" section of the PR is unsupported by the actual changes. Finally, the PR states: > if the Commons team is interested I am happy to share the full report Based on the quality of this PR and the amount of review time it has already consumed, I don't think that would be a productive use of anyone's time. -- 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]
