felix-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Carsten Ziegeler <cziege...@apache.org>
Subject Re: Inventory Printer feedback
Date Sun, 24 Feb 2013 09:58:02 GMT
2013/2/23 Felix Meschberger <fmeschbe@adobe.com>:
> Hi
>
> I am looking into cutting a Web Console release soon, which will require a release of
the "inventory" bundle first. This is the bundle previously called status having been renamed
because the maven module name "status-printer" was not congruent with the bundle symbolic
name o.a.f.status.
>
> I have been looking into this module a bit and would like to share some feedback:
Great!

>
> * IIUIC an InventoryPrinter registered with a .category property will be registered with
the web console as a proxy plugin with the respective property (allowing the InventoryPrinter
to be accessed by its own URL). If the .category property is not set, the default Inventory
category is used. I think we should not have such a .category property and these adapter plugins
should always be registered with the Inventory category.

Actually the category property is used for two purposes - the
Inventory stuff can be used without the web console, that's one of the
main reasons to create this separate bundle in the first place. So I
think it makes sense to have a categorization of printers when using
the inventory API directly (see below).

>
> * Configurability: Should the inventory- URL prefix and the Inventory category be configurable
?
I think the category, yes - as this is usable outside of the web
console - prefix, no

>
> * Should the ZipAttachementProvider really extend the InventoryPrinter interface ? I
rather think it should not extend the InventoryPrinter interface. Rather it should be used
as a "marker with API": When writing the ZIP, the InventoryPrinter services are checked whether
they also implement the ZipAttachementProvider and should be called accordingly.
The ZipAttachmentProvider is in fact an extension of the
InventoryPrinter - so why hide this fact? The implementation always
checks all inventory printers whether they implement this interface,
so I see no benefit of having this a separate marker interface.

>
> * Minor: Source Code Formatting should be adapted to our coding conventions.
>
> * What are the InventoryPrinterHandler and InventoryPrinterManager interfaces used for
? Looks like they are used internally in the Inventory bundle -- are they used externally
? I have the impression, this API is not required.
They can be used by any client code to get inventory printers - so for
example if you want to write a gogo shell extension printing out all
information from the inventory printers, these extension can use the
printer manager to get these things. I've added the handler interface
as this one makes handling the printers easiers by client code and
clearly states that client code should never use the printer services
directly.
Of course one could argue that client code could lookup all printers
and use them directly, however this would duplicate all the logic we
already have implemented and therefore I came up with the
manager/handler API for this.

>
> * API: We should probably tag the interfaces with the BND @ConsumerType and @ProviderType
annotations.
Yepp

>
> * While I think "Inventory" as a name is probably better (and less confusing) than Configuration
Status or Status or Status Printer, I am not really convinced (and still fear the name is
confusing). Maybe some native english speaker has a better name for that ?
Yepp good idea - I never liked the status/status printer name and
after some searching came across "inventory" as a name as this is
nearly used anywhere, so we avoid naming confusing with existing
stuff. But then, I'm not a native speaker either.

Carsten
>
> Regards
> Felix
>
> PS: Releasing the Inventory bundle blocks the Web Console release because the "all-in-one"
profile of the Web Console embeds the Inventory bundle.



-- 
Carsten Ziegeler
cziegeler@apache.org

Mime
View raw message