You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@felix.apache.org by Guillaume Nodet <gn...@gmail.com> on 2010/03/05 16:05:03 UTC
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/
On Fri, Mar 5, 2010 at 15:57, Felix Meschberger <fm...@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).
>> + <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
>
--
Cheers,
Guillaume Nodet
------------------------
Blog: http://gnodet.blogspot.com/
------------------------
Open Source SOA
http://fusesource.com
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/
Posted by Felix Meschberger <fm...@gmail.com>.
Hi,
On 05.03.2010 16:05, Guillaume Nodet wrote:
> On Fri, Mar 5, 2010 at 15:57, Felix Meschberger <fm...@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.
Regards
Felix
>
>>> + <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
>>
>
>
>