jclouds-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrew Gaul <notificati...@github.com>
Subject Re: [jclouds/jclouds] JCLOUDS-1334: Guava 23 compatibility (SimpleTimeLimiter creation) (#1133)
Date Sat, 26 Aug 2017 22:46:30 GMT
andrewgaul requested changes on this pull request.



> +    * @return a new instance of SimpleTimeLimiter that uses executorService
+    */
+   public static SimpleTimeLimiter createSimpleTimeLimiter(ExecutorService executorService)
{
+      try {
+         if (CREATE_STL != null) {
+            return (SimpleTimeLimiter) CREATE_STL.invoke(null, executorService);
+         } else if (CONSTRUCT_STL != null) {
+            return CONSTRUCT_STL.newInstance(executorService);
+         }
+         // fall through
+      } catch (IllegalAccessException iae) {
+         // fall through
+      } catch (InstantiationException ie) {
+         // fall through
+      } catch (InvocationTargetException ite) {
+          throw Throwables.propagate(ite.getCause());

This is not quite right -- you want to propagate the actual exception, not its cause.  Also
this does not propagate the first two exceptions.  Finally ou can just `throw RuntimeException`;
Guava deprecated `Throwables.propagate`.

> @@ -92,22 +92,22 @@
 import com.google.inject.TypeLiteral;
 
 /**
- * 
+ *

Can you remove these spurious changes?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1133#pullrequestreview-58810814
Mime
View raw message