[rfc][icedtea-web] Refactor initializeResources in ResourceDownloader

Lukasz Dracz ldracz at redhat.com
Tue Feb 17 21:51:24 UTC 2015


Hello,


> ----- Original Message -----
> > Hello,
> > 
> > The patch looks good to me !
> > 
> > Only one small nit
> > 
> >          } finally {
> > -            entry.unlock();
> > +             entry.unlock();
> >          }
> > 
> > I think you added an extra space, which is not needed.
> > 
> > Other than that I give it +1 from me
> > Thanks for the patch.
> 
> Hello,
> 
> Mainly for records sake:
> 
> There is one more nit I have fixed with the previous patch.
> 
> +    public static boolean isConnectable(URL location) {
> +        if (location.getProtocol().equals("file")) {
> +            return true;
> +        }
> +
>          try {
> -            if (location.getProtocol().equals("file")) {
> -                return;
> -            }
> -            //Checks the offline/online status of the system.
> +
> OutputController.getLogger().log(OutputController.Level.ERROR_ALL,
> "The host of " + location.toExternalForm() + " file should be located seems
> down, or you are simply offline.");

Good catch !

> This error message should be placed in the catch statement, not in the try.
> My bad. Message has also been altered to be easier to read. s/file should be
> located seems/file seems/

Yeah this seems good to me.

>              InetAddress.getByName(location.getHost());
> -        } catch (UnknownHostException ue) {
> -
> OutputController.getLogger().log(OutputController.Level.ERROR_ALL,
> "The host of " + location.toExternalForm() + " file should be located seems
> down, or you are simply offline.");
> -            JNLPRuntime.setOnlineDetected(false);
> -            return;
> +        } catch (UnknownHostException e) {
> +            return false;
>          }
> -        setOnlineDetected(true);
> +
> +        return true;
> 
> Patch with changes attached.
> 
> 
> Okay to push?

Yes, a +1 from me.

Thank you,
Lukasz Dracz


More information about the distro-pkg-dev mailing list