[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