Github user ahgittin commented on the issue:

    https://github.com/apache/brooklyn-server/pull/485
  
    Agree on all points - thanks.
    
    Best
    Alex
    
    On 15 Dec 2016 17:14, "Aled Sage" <[email protected]> wrote:
    
    > @ahgittin <https://github.com/ahgittin> (cc @neykov
    > <https://github.com/neykov>) I've only had a quick look, but a few
    > initial comments:
    >
    >    1.
    >
    >    Discuss on dev@brooklyn the idea of deploying blueprints via a
    >    directory/zip (rather than a user having to write mvn build stuff to
    >    produce a bundle). That's a major new feature, so should be discussed 
early
    >    rather than doing (some of) the work first - sorry if I missed an email
    >    thread! I strongly agree with adding the feature, but we should
    >    propose/discuss it first.
    >    2.
    >
    >    Discuss on dev@brooklyn about what structure we should support for the
    >    zip files (e.g. CatalogBomScanner.CatalogPopulator I think looks for
    >    catalog.bom and then any brooklyn.libraries it lists). I'm all for
    >    convention-over-configuration, but want us to discuss what those
    >    conventions should be.
    >    3.
    >
    >    You suggested changing BrooklynFeatureEnablement.
    >    FEATURE_LOAD_BUNDLE_CATALOG_BOM to be true - let's discuss that on the
    >    mailing list as well.
    >    4.
    >
    >    You said "not directly useful in Brooklyn yet", but you've added this
    >    to the REST api. What do you mean by "not directly useful"?
    >    5.
    >
    >    I'd move BundleMaker into a utility module, instead of brooklyn-core.
    >    It's nice to force us to keep things more modular, and not to introduce
    >    unnecessary dependencies on brooklyn things when writing library code 
(I'd
    >    get rid of the constructor that takes the ManagementContext, and leave 
it
    >    to the caller to extract the framework).
    >    6.
    >
    >    There's no BundleMakerTest (but BundleMaker is used in
    >    CatalogMakeOsgiBundleTest and CatalogResourceTest). It would be good
    >    to have that, exercising each method.
    >
    > —
    > You are receiving this because you were mentioned.
    > Reply to this email directly, view it on GitHub
    > 
<https://github.com/apache/brooklyn-server/pull/485#issuecomment-267385102>,
    > or mute the thread
    > 
<https://github.com/notifications/unsubscribe-auth/AAeTnCyDBvSG0o4vnqu7VJ0T3e-HF_Xvks5rIXVWgaJpZM4LKyer>
    > .
    >



---
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 [email protected] or file a JIRA ticket
with INFRA.
---

Reply via email to