[RFC][Icedtea-web] Add cache viewer for control panel.

Omair Majid omajid at redhat.com
Mon Dec 20 09:31:53 PST 2010


On 12/20/2010 12:24 PM, Andrew Su wrote:
> On 12/20/2010 11:07 AM, Omair Majid wrote:
>> On 12/20/2010 10:56 AM, Andrew Su wrote:
>>> On 12/17/2010 11:09 AM, Omair Majid wrote:
>>>> On 12/15/2010 03:30 PM, Andrew Su wrote:
>>>>>> All these changes have now been added + fixed in this updated patch.
>>>>>>
>>>>>> Changelog:
>>>>>>
>>>>>> Added a cache viewer for the control panel.
>>>>>> *
>>>>>> netx/net/sourceforge/jnlp/controlpanel/TemporaryInternetFilesPanel.java:
>>>>>>
>>>>>>
>>>>>> (addComponents): Changed buttons to open cache viewer.
>>>>>> * netx/net/sourceforge/jnlp/resources/Messages.properties: Added text
>>>>>> used by the cache viewer
>>>>>> * netx/net/sourceforge/jnlp/cache/CacheDirectory.java,
>>>>>> netx/net/sourceforge/jnlp/cache/DirectoryNode.java,
>>>>>> netx/net/sourceforge/jnlp/controlpanel/CachePane.java,
>>>>>> netx/net/sourceforge/jnlp/controlpanel/CacheViewer.java: New classes.
>>>>>>
>>>>>> Questions? Comments? Concerns?
>>>>>>
>>>>
>>>> Overall it looks fine to me. I only have two concerns:
>>>>
>>>> Please fix the license in files that you are adding to
>>>> net.sourceforge.jnlp.cache.*. It should be LGPL, not GPL.
>>>>
>>>> Please avoid adding in extra calls to UIManager.setLookAndFeel().
>>>> Calling it multiple times doesnt make it any more system-like :) This
>>>> call should be made just once - preferably in ControLPanel's main
>>>> method - before any GUI components have been created.
>>>>
>>>> Other than those issues above, the patch looks fine to me. It would be
>>>> nice to get rid of extra whitespace characters after the end of lines
>>>> too, but I dont think that's a big deal (ctrl-shift+f in eclipse
>>>> should fix it).
>>>>
>>>> Cheers,
>>>> Omair
>>> Hi Omair,
>>>
>>> This is the updated patch which doesn't set the look and feel, and
>>> licences.
>>>
>>> Ok for commit?
>>>
>>
>> Err... I suppose I wasnt very clear. We want a GPL license on the
>> control panel itself - since it is a stand alone binary (and can not
>> be used as a library). Everything else, which may be used by
>> closed-source programs, should be GPL+Classpath.
>>
>> The license for these files should be GPL+Classpath (_not_ GPL):
>> netx/net/sourceforge/jnlp/cache/CacheDirectory.java
>> netx/net/sourceforge/jnlp/cache/DirectoryNode.java
>>
>> The licence on these files should be GPL (_not_ GPL+Classpath):
>> netx/net/sourceforge/jnlp/controlpanel/CachePane.java
>> netx/net/sourceforge/jnlp/controlpanel/CacheViewer.java
>>
> Okay, I changed the headers again.
> How's this one?
>

A minor nitpick: ButApply is defined twice in Messages.properties file. 
Please remove the copy at the end of the file. Other than that, the 
patch looks good to me. Please go ahead and commit with the suggested 
change to HEAD.

Cheers,
Omair



More information about the distro-pkg-dev mailing list