felix-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Felix Meschberger <fmesc...@gmail.com>
Subject Re: svn commit: r919175 - in /felix/trunk/webconsole: ./ src/main/java/org/apache/felix/webconsole/internal/core/ src/main/java/org/apache/felix/webconsole/internal/obr/ src/main/resources/res/ui/ src/main/resources/templates/
Date Fri, 05 Mar 2010 15:11:47 GMT

On 05.03.2010 16:05, Guillaume Nodet wrote:
> On Fri, Mar 5, 2010 at 15:57, Felix Meschberger <fmeschbe@gmail.com> wrote:
>> Hi,
>> This patch has a few issues IMHO:
>> On 04.03.2010 21:54, gnodet@apache.org wrote:
>>> Added:
>>>     felix/trunk/webconsole/src/main/resources/res/ui/jquery.uuid.js
>> For the jquery.uuid.js plugin we need the NOTICE entry and LICENSE file
>>> -    // Define a constant of that name to prevent NoClassDefFoundError in
>>> -    // updateFromOBR trying to load the class with RepositoryAdmin.class
>>> -    private static final String REPOSITORY_ADMIN_NAME = "org.osgi.service.obr.RepositoryAdmin";
>> The reason for me to define this as a string is to prevent an exception
>> if the RepositoryAdmin service API cannot be imported because there is
>> no API provider bundle installed.
> Well, I'd have no problem if the class would not use the RepositoryAdmin.
> Given it *is* used, using a string here does not make sense.
> In addition, the webconsole starts correctly if the package is not available.
> I've checked that (though I may have missed something).

You are right. It looks like the plugin cannot be activated if the
RepositoryAdmin API is not available. Previously the plugin could be
activated and displayed.

And  intererstingly, just defining a variable with the RepositoryAdmin
type does not constitute a requirement to load the class, thus

   RepositoryAdmin admin = getService("RepositoryAdmin");

just silently sets the admin field to null if neither the type nor the
actual service is avaible. Thus the string-thing worked perfectly.

But it is ok for me, too that the plugin is not activated at all if the
API is not available. So lets keep it like it is right now.


>>> +        <a href="obr?list=a">A</a>
>> While chances are, the label will never change, I nevertheless think, it
>> is not optimal to hard code this here.
>> This could probably be written as
>>           <a href="${pluginRoot}?list=a">A</a>
>> to ensure the correct URL is used regardless of how the actual bundle is
>> configured/set up/coded.
> Yeah, makes sense.
>> Also there is a function added stemming from the Mozilla project. This
>> should probably also be noted in the NOTICE/LICENSE files, right ?
> Will update the NOTICE/LICENSE files for this function and the jquery.uuid.js
> Thx for the reminder.
>> Sorry, to bother.
>> Regards
>> Felix

View raw message