You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "Pankraz76 (via GitHub)" <gi...@apache.org> on 2025/05/31 09:55:44 UTC

[PR] Fix 'string builder buffer' can be replaced with 'string' [maven]

Pankraz76 opened a new pull request, #2415:
URL: https://github.com/apache/maven/pull/2415

   
   
   
   
   <img width="1133" alt="image" src="https://github.com/user-attachments/assets/b9f95200-cb5e-4dd2-88b8-d65e95182500" />
   


-- 
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-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix 'string builder buffer' can be replaced with 'string' [maven]

Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on code in PR #2415:
URL: https://github.com/apache/maven/pull/2415#discussion_r2117682942


##########
compat/maven-compat/src/main/java/org/apache/maven/artifact/repository/layout/FlatRepositoryLayout.java:
##########
@@ -62,19 +62,11 @@ public String pathOf(Artifact artifact) {
     }
 
     public String pathOfLocalRepositoryMetadata(ArtifactMetadata metadata, ArtifactRepository repository) {

Review Comment:
   might conflict with: 
   
   - https://github.com/apache/maven/pull/2402
   
   



-- 
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-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix 'string builder buffer' can be replaced with 'string' [maven]

Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #2415:
URL: https://github.com/apache/maven/pull/2415#discussion_r2117694456


##########
compat/maven-artifact/src/main/java/org/apache/maven/artifact/repository/ArtifactRepositoryPolicy.java:
##########
@@ -132,15 +132,7 @@ public boolean checkOutOfDate(Date lastModified) {
 
     @Override
     public String toString() {
-        StringBuilder buffer = new StringBuilder(64);
-        buffer.append("{enabled=");
-        buffer.append(enabled);
-        buffer.append(", checksums=");
-        buffer.append(checksumPolicy);
-        buffer.append(", updates=");
-        buffer.append(updatePolicy);
-        buffer.append('}');
-        return buffer.toString();
+        return "{enabled=" + enabled + ", checksums=" + checksumPolicy + ", updates=" + updatePolicy + '}';

Review Comment:
   don't do this; it's actively bad



-- 
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-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix 'string builder buffer' can be replaced with 'string' [maven]

Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on code in PR #2415:
URL: https://github.com/apache/maven/pull/2415#discussion_r2117683011


##########
compat/maven-compat/src/main/java/org/apache/maven/artifact/repository/layout/FlatRepositoryLayout.java:
##########
@@ -62,19 +62,11 @@ public String pathOf(Artifact artifact) {
     }
 
     public String pathOfLocalRepositoryMetadata(ArtifactMetadata metadata, ArtifactRepository repository) {
-        return pathOfRepositoryMetadata(metadata.getLocalFilename(repository));
-    }
-
-    private String pathOfRepositoryMetadata(String filename) {
-        StringBuilder path = new StringBuilder(128);
-
-        path.append(filename);
-
-        return path.toString();
+        return metadata.getLocalFilename(repository);
     }
 
     public String pathOfRemoteRepositoryMetadata(ArtifactMetadata metadata) {
-        return pathOfRepositoryMetadata(metadata.getRemoteFilename());
+        return metadata.getRemoteFilename();

Review Comment:
   why no default impl in interface its pure domain logic?
   - https://github.com/apache/maven/pull/2402



-- 
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-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix 'string builder buffer' can be replaced with 'string' [maven]

Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on code in PR #2415:
URL: https://github.com/apache/maven/pull/2415#discussion_r2117681105


##########
compat/maven-compat/src/main/java/org/apache/maven/artifact/repository/layout/FlatRepositoryLayout.java:
##########
@@ -66,11 +66,8 @@ public String pathOfLocalRepositoryMetadata(ArtifactMetadata metadata, ArtifactR
     }
 
     private String pathOfRepositoryMetadata(String filename) {

Review Comment:
   this method seems kind of 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.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix 'string builder buffer' can be replaced with 'string' [maven]

Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on code in PR #2415:
URL: https://github.com/apache/maven/pull/2415#discussion_r2117688353


##########
compat/maven-compat/src/test/java/org/apache/maven/project/AbstractMavenProjectTestCase.java:
##########
@@ -112,10 +112,8 @@ protected static File getFileForClasspathResource(String resource)
     protected ArtifactRepository getLocalRepository() throws Exception {
         ArtifactRepositoryLayout repoLayout = getContainer().lookup(ArtifactRepositoryLayout.class);
 
-        ArtifactRepository r = repositorySystem.createArtifactRepository(
+        return repositorySystem.createArtifactRepository(
                 "local", "file://" + getLocalRepositoryPath().getAbsolutePath(), repoLayout, null, null);
-
-        return r;

Review Comment:
   - https://github.com/pmd/pmd/issues/5770



-- 
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-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix 'string builder buffer' can be replaced with 'string' [maven]

Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on code in PR #2415:
URL: https://github.com/apache/maven/pull/2415#discussion_r2117680714


##########
compat/maven-compat/src/main/java/org/apache/maven/artifact/repository/layout/FlatRepositoryLayout.java:
##########
@@ -66,11 +66,8 @@ public String pathOfLocalRepositoryMetadata(ArtifactMetadata metadata, ArtifactR
     }
 
     private String pathOfRepositoryMetadata(String filename) {
-        StringBuilder path = new StringBuilder(128);

Review Comment:
   yes.



-- 
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-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix 'string builder buffer' can be replaced with 'string' [maven]

Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on code in PR #2415:
URL: https://github.com/apache/maven/pull/2415#discussion_r2117695649


##########
compat/maven-artifact/src/main/java/org/apache/maven/artifact/repository/ArtifactRepositoryPolicy.java:
##########
@@ -132,15 +132,7 @@ public boolean checkOutOfDate(Date lastModified) {
 
     @Override
     public String toString() {
-        StringBuilder buffer = new StringBuilder(64);
-        buffer.append("{enabled=");
-        buffer.append(enabled);
-        buffer.append(", checksums=");
-        buffer.append(checksumPolicy);
-        buffer.append(", updates=");
-        buffer.append(updatePolicy);
-        buffer.append('}');
-        return buffer.toString();
+        return "{enabled=" + enabled + ", checksums=" + checksumPolicy + ", updates=" + updatePolicy + '}';

Review Comment:
   before it was optimized, right?



-- 
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-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix 'string builder buffer' can be replaced with 'string' [maven]

Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on code in PR #2415:
URL: https://github.com/apache/maven/pull/2415#discussion_r2117687298


##########
compat/maven-compat/src/main/java/org/apache/maven/artifact/repository/layout/FlatRepositoryLayout.java:
##########
@@ -66,11 +66,8 @@ public String pathOfLocalRepositoryMetadata(ArtifactMetadata metadata, ArtifactR
     }
 
     private String pathOfRepositoryMetadata(String filename) {
-        StringBuilder path = new StringBuilder(128);

Review Comment:
   this method is interesting @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.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix 'string builder buffer' can be replaced with 'string' [maven]

Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 closed pull request #2415: Fix 'string builder buffer' can be replaced with 'string' 
URL: https://github.com/apache/maven/pull/2415


-- 
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-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org