felix-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Felix Meschberger <fmesc...@adobe.com>
Subject Re: Inventory Printer feedback
Date Fri, 01 Mar 2013 15:57:18 GMT
Hi

Thanks for the updates. I just have committed a unit test for PrinterMode and reformatted
the source code to match our conventions.

Unless there is more discussion on this new module, I think I will consider further open issues
of the Web Console and then proceeed to cut releases of both modules next week.

Regards
Felix

Am 26.02.2013 um 12:08 schrieb Carsten Ziegeler:

> Hi,
> 
> after some further thinking, I did now the following changes:
> - remove all usage of Java 5 features (enum, generics) and support 1.4
> as target (same as the web console does) (therefore we can't use the
> bnd annotations)
> - removed the manager and handler interfaces - right now these are not
> needed and we can add them once we really need them
> - without the manager, category doesn't make sense, therefore I
> removed this as well
> - ZipAttachementProvider does not inherit from InventoryPrinter anymore
> - The printers are registered with the category "Status" for the web console
> 
> Regards
> Carsten
> 
> 2013/2/24 Carsten Ziegeler <cziegeler@apache.org>:
>> 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
> 
> 
> 
> -- 
> Carsten Ziegeler
> cziegeler@apache.org


--
Felix Meschberger | Principal Scientist | Adobe








Mime
View raw message