massdosage commented on a change in pull request #1154:
URL: https://github.com/apache/iceberg/pull/1154#discussion_r448920761



##########
File path: core/src/main/java/org/apache/iceberg/BaseMetastoreCatalog.java
##########
@@ -277,7 +277,7 @@ private static void deleteFiles(FileIO io, 
Set<ManifestFile> allManifests) {
               }
             }
           } catch (IOException e) {
-            throw new RuntimeIOException(e, "Failed to read manifest file: " + 
manifest.path());
+            throw new UncheckedIOException("Failed to read manifest file: " + 
manifest.path(), e);

Review comment:
       OK, the above context makes sense, but I still think the principle of 
"least surprises" would lean me to not do this and just make sure people write 
good messages when they create exceptions. Anyway, at some point I think it 
would be good to  to have a DEVELOPERS.md (or something equivalent) for getting 
new developers up to speed with things like the above where the project may 
differ from what they are used to.
   
   I'll update this PR for `RuntimeIOException` to extend 
`UncheckedIOException`.




----------------------------------------------------------------
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:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to