[GitHub] [maven-shared-utils] michael-o commented on a change in pull request #41: [MSHARED-860] deprecate PropertyUtils constructor and clean up docs

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

michael-o commented on a change in pull request #41:
URL: https://github.com/apache/maven-shared-utils/pull/41#discussion_r432415800



##########
File path: src/main/java/org/apache/maven/shared/utils/PropertyUtils.java
##########
@@ -89,22 +89,21 @@ public static Properties loadProperties( @Nonnull File file )
 
     /**
      * @param is {@link InputStream}
-     * @return The loaded properties.
-     * @deprecated As of 3.1.0, please use method {@link #loadOptionalProperties(java.io.InputStream)}. This method
+     * @return the loaded properties
+     * @deprecated use {@link #loadOptionalProperties(java.io.InputStream)} instead. This method
      *             should not be used as it suppresses exceptions silently when loading properties fails.
      */
     @Deprecated
     public static Properties loadProperties( @Nullable InputStream is )
     {
         try
         {
-            // to make this the same behaviour as the others we should really return null on any error
             Properties result = new Properties();
             if ( is != null )
             {
-                try
+                try ( InputStream in = is )

Review comment:
       I don't agree with this because have not opened this stream and it is not our task to close it. The one who opens the stream must close it.




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