[GitHub] incubator-brooklyn pull request: Initial OSGI work

[aledsage <gi...@git.apache.org>]

Github user aledsage commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/31#discussion_r14378460
  
    --- Diff: api/src/main/java/brooklyn/catalog/BrooklynCatalog.java ---
    @@ -4,45 +4,49 @@
     
     public interface BrooklynCatalog {
     
    -    /** finds the item with the given ID, or null if not found */
    +    /** @return The item with the given ID, or null if not found */
         CatalogItem<?> getCatalogItem(String id);
    -    
    +
    +
         /** variant of {@link #getCatalogItem(String)} which checks (and casts) type for convenience
          * (returns null if type does not match) */
         <T> CatalogItem<T> getCatalogItem(Class<T> type, String id);
    -    
    -    /** returns all items in the catalog */
    +
    +    /** @return All items in the catalog */
         <T> Iterable<CatalogItem<T>> getCatalogItems();
    +
         /** convenience for filtering items in the catalog; see CatalogPredicates for useful filters */
         <T> Iterable<CatalogItem<T>> getCatalogItems(Predicate<? super CatalogItem<T>> filter);
     
    -    /** returns the classloader which should be used to load classes and entities;
    +    /** @return The classloader which should be used to load classes and entities;
          * this includes all the catalog's classloaders in the right order */
         public ClassLoader getRootClassLoader();
    -    
    +
         /** throws exceptions if any problems */
         <T> Class<? extends T> loadClass(CatalogItem<T> item);
         <T> Class<? extends T> loadClassByType(String typeName, Class<T> typeClass);
    -    
    -    /** adds an item to the 'manual' catalog;
    -     * this does not update the classpath or have a record to the java Class,
    -     * so callers of this method will typically also need to call 
    -     * {@link #addToClasspath(String)} or {@link #addToClasspath(ClassLoader)} */
    +
    +    /**
    +     * adds an item to the 'manual' catalog;
    +     * this does not update the classpath or have a record to the java Class
    +     *
    +     * @deprecated since 0.7.0 Construct catalogs with OSGi bundles instead
    +     */
    +    @Deprecated
    --- End diff --
    
    Will we instead pass the yaml in, so have a `addItem(String yaml)`?
    Or should we also / instead support `addItem(CatalogItem)` where the CatalogItem can include the yaml, as well as the extracted metadata about it?
    (Note it's currently not obvious how to create a CatalogItem to use this - one has to instantiate+set a CatalogEntityItemDto, which is buried in an `.internal` package.)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---