[RFC][icedtea-web] Small code cleanup changes: fixing some code cloning, misleading comments & parameters etc

Omair Majid omajid at redhat.com
Thu May 17 14:44:14 PDT 2012


Hi Adam,

On 05/17/2012 05:19 PM, Adam Domurad wrote:
> This incorporates some changes I made to reduce code cloning on a
> patch that I have to reconsider partially, as well as some comment
> clean ups etc. Generally contains small code cleanup changes, fixing
> some code cloning, misleading comments & parameters etc

Comments in-line, below.

> diff --git a/netx/net/sourceforge/jnlp/browser/BrowserAwareProxySelector.java b/netx/net/sourceforge/jnlp/browser/BrowserAwareProxySelector.java
> --- a/netx/net/sourceforge/jnlp/browser/BrowserAwareProxySelector.java
> +++ b/netx/net/sourceforge/jnlp/browser/BrowserAwareProxySelector.java
> @@ -198,12 +198,14 @@ public class BrowserAwareProxySelector e
>                  if (optionDescription == null) {
>                      optionDescription = "Automatic";
>                  }
> +                break;
>              case BROWSER_PROXY_TYPE_SYSTEM:
>                  // means use $http_proxy, $ftp_proxy etc.
>                  // TODO implement env vars if possible
>                  if (optionDescription == null) {
>                      optionDescription = "System";
>                  }
> +                break;
>              default:
>                  if (optionDescription == null) {
>                      optionDescription = "Unknown";

Please don't do this. The fall-through here is intentional. It wont do
much harm, but it will leave out extra information for debugging. I
wouldn't mind if you added a comment here indicating this, though.
Something like:

/* fallthrough */

> diff --git a/plugin/icedteanp/java/sun/applet/AppletSecurityContextManager.java b/plugin/icedteanp/java/sun/applet/AppletSecurityContextManager.java
> --- a/plugin/icedteanp/java/sun/applet/AppletSecurityContextManager.java
> +++ b/plugin/icedteanp/java/sun/applet/AppletSecurityContextManager.java
> @@ -1,4 +1,4 @@
> -/* VoidPluginCallRequest -- represent Java-to-JavaScript requests
> +/* AppletSecurityContextManager TODO: describe this class
>     Copyright (C) 2008  Red Hat
>  
>  This file is part of IcedTea.
> diff --git a/plugin/icedteanp/java/sun/applet/PluginCallRequestFactory.java b/plugin/icedteanp/java/sun/applet/PluginCallRequestFactory.java
> --- a/plugin/icedteanp/java/sun/applet/PluginCallRequestFactory.java
> +++ b/plugin/icedteanp/java/sun/applet/PluginCallRequestFactory.java
> @@ -1,4 +1,4 @@
> -/* VoidPluginCallRequest -- represent Java-to-JavaScript requests
> +/* PluginCallRequestFactory  TODO: describe this class
>     Copyright (C) 2008  Red Hat
>  
>  This file is part of IcedTea.
> diff --git a/plugin/icedteanp/java/sun/applet/PluginClassLoader.java b/plugin/icedteanp/java/sun/applet/PluginClassLoader.java
> --- a/plugin/icedteanp/java/sun/applet/PluginClassLoader.java
> +++ b/plugin/icedteanp/java/sun/applet/PluginClassLoader.java
> @@ -1,4 +1,4 @@
> -/* VoidPluginCallRequest -- represent Java-to-JavaScript requests
> +/* PluginClassLoader
>     Copyright (C) 2008  Red Hat
>  
>  This file is part of IcedTea.
> diff --git a/plugin/icedteanp/java/sun/applet/PluginDebug.java b/plugin/icedteanp/java/sun/applet/PluginDebug.java
> --- a/plugin/icedteanp/java/sun/applet/PluginDebug.java
> +++ b/plugin/icedteanp/java/sun/applet/PluginDebug.java
> @@ -1,4 +1,4 @@
> -/* VoidPluginCallRequest -- represent Java-to-JavaScript requests
> +/* PluginDebug  TODO: describe this class
>     Copyright (C) 2008  Red Hat
>  
>  This file is part of IcedTea.
> diff --git a/plugin/icedteanp/java/sun/applet/PluginException.java b/plugin/icedteanp/java/sun/applet/PluginException.java
> --- a/plugin/icedteanp/java/sun/applet/PluginException.java
> +++ b/plugin/icedteanp/java/sun/applet/PluginException.java
> @@ -1,4 +1,4 @@
> -/* VoidPluginCallRequest -- represent Java-to-JavaScript requests
> +/* PluginException TODO: describe this class
>     Copyright (C) 2008  Red Hat
>  
>  This file is part of IcedTea.
> diff --git a/plugin/icedteanp/java/sun/applet/PluginMain.java b/plugin/icedteanp/java/sun/applet/PluginMain.java
> --- a/plugin/icedteanp/java/sun/applet/PluginMain.java
> +++ b/plugin/icedteanp/java/sun/applet/PluginMain.java
> @@ -1,4 +1,4 @@
> -/* VoidPluginCallRequest -- represent Java-to-JavaScript requests
> +/* PluginMain  TODO: describe this class
>     Copyright (C) 2008  Red Hat
>  
>  This file is part of IcedTea.
> diff --git a/plugin/icedteanp/java/sun/applet/PluginMessageConsumer.java b/plugin/icedteanp/java/sun/applet/PluginMessageConsumer.java
> --- a/plugin/icedteanp/java/sun/applet/PluginMessageConsumer.java
> +++ b/plugin/icedteanp/java/sun/applet/PluginMessageConsumer.java
> @@ -1,4 +1,4 @@
> -/* VoidPluginCallRequest -- represent Java-to-JavaScript requests
> +/* PluginMessageConsumer  TODO: describe this class
>     Copyright (C) 2008  Red Hat
>  
>  This file is part of IcedTea.
> diff --git a/plugin/icedteanp/java/sun/applet/PluginMessageHandlerWorker.java b/plugin/icedteanp/java/sun/applet/PluginMessageHandlerWorker.java
> --- a/plugin/icedteanp/java/sun/applet/PluginMessageHandlerWorker.java
> +++ b/plugin/icedteanp/java/sun/applet/PluginMessageHandlerWorker.java
> @@ -1,4 +1,4 @@
> -/* VoidPluginCallRequest -- represent Java-to-JavaScript requests
> +/* PluginMessageHandler  TODO: describe this class
>     Copyright (C) 2008  Red Hat
>  
>  This file is part of IcedTea.
> diff --git a/plugin/icedteanp/java/sun/applet/PluginStreamHandler.java b/plugin/icedteanp/java/sun/applet/PluginStreamHandler.java
> --- a/plugin/icedteanp/java/sun/applet/PluginStreamHandler.java
> +++ b/plugin/icedteanp/java/sun/applet/PluginStreamHandler.java
> @@ -1,4 +1,4 @@
> -/* VoidPluginCallRequest -- represent Java-to-JavaScript requests
> +/* PluginStreamHandler
>     Copyright (C) 2008  Red Hat
>  
>  This file is part of IcedTea.
> diff --git a/plugin/icedteanp/java/sun/applet/RequestQueue.java b/plugin/icedteanp/java/sun/applet/RequestQueue.java
> --- a/plugin/icedteanp/java/sun/applet/RequestQueue.java
> +++ b/plugin/icedteanp/java/sun/applet/RequestQueue.java
> @@ -1,4 +1,4 @@
> -/* VoidPluginCallRequest -- represent Java-to-JavaScript requests
> +/* RequestQueue TODO: describe this class
>     Copyright (C) 2008  Red Hat
>  
>  This file is part of IcedTea.
> 

Could you actually fill in the TODOs befor commiting?

Cheers,
Omair



More information about the distro-pkg-dev mailing list