You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2021/01/17 19:00:22 UTC
[GitHub] [maven-artifact-plugin] elharo opened a new pull request #7: prefer Apache Commons StringUtils
elharo opened a new pull request #7:
URL: https://github.com/apache/maven-artifact-plugin/pull/7
@hboutemy
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-artifact-plugin] bmarwell commented on pull request #7: prefer Apache Commons StringUtils
Posted by GitBox <gi...@apache.org>.
bmarwell commented on pull request #7:
URL: https://github.com/apache/maven-artifact-plugin/pull/7#issuecomment-763384330
I'd say `-1` to this one. Either create a `maven-lang` lib for the same reason as guava, commons-lang, etc. exist, or just wait for Java 8. @elharo no offense intended, but I honestly think (while I fully agree with what you mentioned earlier!) pulling in another dependency replacing a not-removable dependency is not worth it. Even if more changes like this are to come.
It is well-tested enough as this code existed for years and did not break.
So, Maven 4 + Java 8 is not far away. Lets wait a few months and do a *real* java8 clean up then, wdyt?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-artifact-plugin] hboutemy commented on pull request #7: prefer Apache Commons StringUtils
Posted by GitBox <gi...@apache.org>.
hboutemy commented on pull request #7:
URL: https://github.com/apache/maven-artifact-plugin/pull/7#issuecomment-761889739
we made Maven Shared Utils for that
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-artifact-plugin] rmannibucau commented on pull request #7: prefer Apache Commons StringUtils
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on pull request #7:
URL: https://github.com/apache/maven-artifact-plugin/pull/7#issuecomment-762973071
@michael-o most of these utilities are in java 8 or are trivial (like 1 or 2 calls) so the need to argument about a 3rd party disappear for all modules.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-artifact-plugin] asfgit closed pull request #7: prefer Apache Commons StringUtils
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #7:
URL: https://github.com/apache/maven-artifact-plugin/pull/7
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-artifact-plugin] hboutemy closed pull request #7: prefer Apache Commons StringUtils
Posted by GitBox <gi...@apache.org>.
hboutemy closed pull request #7:
URL: https://github.com/apache/maven-artifact-plugin/pull/7
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-artifact-plugin] hboutemy closed pull request #7: prefer Apache Commons StringUtils
Posted by GitBox <gi...@apache.org>.
hboutemy closed pull request #7:
URL: https://github.com/apache/maven-artifact-plugin/pull/7
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-artifact-plugin] elharo commented on a change in pull request #7: prefer Apache Commons StringUtils
Posted by GitBox <gi...@apache.org>.
elharo commented on a change in pull request #7:
URL: https://github.com/apache/maven-artifact-plugin/pull/7#discussion_r560906065
##########
File path: src/main/java/org/apache/maven/plugins/artifact/buildinfo/BuildinfoMojo.java
##########
@@ -350,8 +351,8 @@ private void compareWithReference( Map<Artifact, String> artifacts, File referen
p.println( "version=" + project.getVersion() );
p.println( "ok=" + ok );
p.println( "ko=" + ko );
- p.println( "okFiles=\"" + StringUtils.join( okFilenames.iterator(), " " ) + '"' );
- p.println( "koFiles=\"" + StringUtils.join( koFilenames.iterator(), " " ) + '"' );
+ p.println( "okFiles=\"" + StringUtils.join( okFilenames, " " ) + '"' );
Review comment:
Notice how Commons-lang StringUtils.join supports Iterable, from Java 1.5; and maven-shared-utils StringUtils.join only supports Iterator from Java 1.2. It's a minor detail, but it does show the relative amounts of maintenance and attention these two products receive.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-artifact-plugin] michael-o commented on pull request #7: prefer Apache Commons StringUtils
Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #7:
URL: https://github.com/apache/maven-artifact-plugin/pull/7#issuecomment-762829347
I fully agree with @elharo
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-artifact-plugin] hboutemy commented on pull request #7: prefer Apache Commons StringUtils
Posted by GitBox <gi...@apache.org>.
hboutemy commented on pull request #7:
URL: https://github.com/apache/maven-artifact-plugin/pull/7#issuecomment-761867766
we rare on Java 7: as written on the dev@ ML; it is key to rebuild some releases to have a maven-artifact-plugin with Java 7
and I'm -1 on adding a new dependency for 1 stupid utility: I'd love that we work on really improving things instead of playing with such style changes
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-artifact-plugin] elharo commented on pull request #7: prefer Apache Commons StringUtils
Posted by GitBox <gi...@apache.org>.
elharo commented on pull request #7:
URL: https://github.com/apache/maven-artifact-plugin/pull/7#issuecomment-763553832
Maven 4 is likely more than a few months away, and even if it arrives we'll be supporting Maven 3 for years to come. We still have code in the project meant to support Maven **2** and that was over a decade ago.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-artifact-plugin] michael-o commented on pull request #7: prefer Apache Commons StringUtils
Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #7:
URL: https://github.com/apache/maven-artifact-plugin/pull/7#issuecomment-762865807
>
>
> Lets validate we move to j8 for that too, then all these utilities are useless.
What do you mean by that?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-artifact-plugin] rmannibucau commented on pull request #7: prefer Apache Commons StringUtils
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on pull request #7:
URL: https://github.com/apache/maven-artifact-plugin/pull/7#issuecomment-762854674
Lets validate we move to j8 for that too, then all these utilities are useless.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-artifact-plugin] elharo commented on pull request #7: prefer Apache Commons StringUtils
Posted by GitBox <gi...@apache.org>.
elharo commented on pull request #7:
URL: https://github.com/apache/maven-artifact-plugin/pull/7#issuecomment-761878409
There's likely more from apache commons lang coming. One piece at a time.
The bottom line is that commons lang is well tested, documented, and maintained. Plexus isn't. We should avoid it where we can.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-artifact-plugin] elharo commented on pull request #7: prefer Apache Commons StringUtils
Posted by GitBox <gi...@apache.org>.
elharo commented on pull request #7:
URL: https://github.com/apache/maven-artifact-plugin/pull/7#issuecomment-762820234
Yes, that's why maven-shared-utils was made. And plexus-utils. And Apache commons lang. And Guava. Programmers seem really fond of building utility libraries to do the same hundred or so not-especially-difficult operations. We should standardize on one, and apache commons lang is by far the best maintained such library already in our tree. (Guava's good too but is generally not common in Apache projects. It also has some pretty bad version conflicts.)
maven-shared-utils should be used for methods that are maven specific in some way, not general string and file manipulation utilities. Ultimately I want to deprecate and remove all such general purpose methods from maven-shared-utils so we have less generic utility code to maintain and can focus on Maven itself.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-artifact-plugin] hboutemy commented on pull request #7: prefer Apache Commons StringUtils
Posted by GitBox <gi...@apache.org>.
hboutemy commented on pull request #7:
URL: https://github.com/apache/maven-artifact-plugin/pull/7#issuecomment-763046022
I don't have time to play on that: working on real plugin enhancements is more useful than personal taste changes
if such changes is the only feedback I get, I don't need feedback, thank you
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-artifact-plugin] asfgit closed pull request #7: prefer Apache Commons StringUtils
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #7:
URL: https://github.com/apache/maven-artifact-plugin/pull/7
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-artifact-plugin] rmannibucau commented on pull request #7: prefer Apache Commons StringUtils
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on pull request #7:
URL: https://github.com/apache/maven-artifact-plugin/pull/7#issuecomment-761864479
Since we are java 8 now we can use String.join but if java 7 is of any usefulness Id say a join does not justify a dependency, in particular when it is already avaioable in the stack (we must not be greedy IMHO).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org