[GitHub] [maven] garydgregory commented on a change in pull request #669: Use try-with-resources

[GitBox <gi...@apache.org>]

garydgregory commented on a change in pull request #669:
URL: https://github.com/apache/maven/pull/669#discussion_r800233008



##########
File path: maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java
##########
@@ -216,9 +216,10 @@ private PluginDescriptor extractPluginDescriptor( Artifact pluginArtifact, Plugi
 
                     if ( pluginDescriptorEntry != null )
                     {
-                        InputStream is = pluginJar.getInputStream( pluginDescriptorEntry );
-
-                        pluginDescriptor = parsePluginDescriptor( is, plugin, pluginFile.getAbsolutePath() );
+                        try ( InputStream is = pluginJar.getInputStream( pluginDescriptorEntry ) )

Review comment:
       The issue, from my POV, is that I think it is best for the method that allocates a resource to release it, and to so do "finally" with a try-with-resources statement. There are exceptions of course. What I personally do not favor is the style that I see here and there in Maven where one method allocates a stream and another method deep in the code path closes the stream; to me, this is an anti-pattern.




-- 
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