Giovds commented on code in PR #1105:
URL: https://github.com/apache/maven/pull/1105#discussion_r1191651779


##########
maven-core/src/main/java/org/apache/maven/internal/transformation/ConsumerPomArtifactTransformer.java:
##########
@@ -77,13 +80,34 @@ public void injectTransformedArtifacts(MavenProject 
project, RepositorySystemSes
                 Files.createDirectories(buildDir);
                 generatedFile = Files.createTempFile(buildDir, 
CONSUMER_POM_CLASSIFIER, "pom");
             }
+
+            removeOldConsumerPomFiles(generatedFile);
+
             project.addAttachedArtifact(new ConsumerPomArtifact(project, 
generatedFile, session));
         } else if (project.getModel().isRoot()) {
             throw new IllegalStateException(
                     "The use of the root attribute on the model requires the 
buildconsumer feature to be active");
         }
     }
 
+    private void removeOldConsumerPomFiles(Path generatedFile) throws 
IOException {
+        List<Path> oldConsumerPomFiles;

Review Comment:
   I had a couple of reasons for it. The first reason for me is readability (as 
I'll show down below). 
   
   The second is that I don't think its a good practice to wrap the checked 
exception into a runtime exception (also introducing more code or a wrapper 
'delete' method). Especially since `injectTransformedArtifacts()` already 
throws a checked exception which is probably accounted for somewhere at some 
point.
   
   As far as I know when you rewrite the stream to something like 
`stream.filter(path -> /*filter*/ }).forEach(Files::delete);` you either have 
to wrap the checked exception thrown by `Files.delete(path)` into a runtime 
exception:
   ```java
       private void delete( Path path ) {
           try { Files.delete( path ); }
           catch ( IOException e ) { throw new RuntimeException("", e); }
       }
       
       /* obfuscated*/
       
       stream.filter(path -> /*filter*/ })
              .forEach(this::delete);
   ```
   Or handle it:
   ```java
   stream.filter(path -> /*filter*/ })
              .forEach(path -> {
                   try { Files.delete(path); }
                   catch (IOException ex) { /*handle the exception*/ }
              });
   ```



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

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

Reply via email to