[GitHub] brooklyn-server issue #340: ConfigMap refactor, respecting inheritance

2016-09-22 Thread ahgittin
Github user ahgittin commented on the issue: https://github.com/apache/brooklyn-server/pull/340 Big thank you @aledsage ! I'll fix that last thing you spotted and merged. I expect there are untested corner cases -- there are so many -- but things are far better tested and imp

[GitHub] brooklyn-server issue #340: ConfigMap refactor, respecting inheritance

2016-09-22 Thread aledsage
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-server/pull/340 Thanks @ahgittin - finished reviewing. Overall I'm happy with this. I agree that it feels very complicated (and some of the code is quite subtle, so could do with a more thorough re

[GitHub] brooklyn-server issue #340: ConfigMap refactor, respecting inheritance

2016-09-22 Thread aledsage
Github user aledsage commented on the issue: https://github.com/apache/brooklyn-server/pull/340 Is there a test of `NOT_REINHERITABLE` for runtime inheritance with app->child->grandchild, where the value is set on `app`, the `child` consumes it, and the `grandchild` does not see it?

[GitHub] brooklyn-server issue #340: ConfigMap refactor, respecting inheritance

2016-09-22 Thread ahgittin
Github user ahgittin commented on the issue: https://github.com/apache/brooklyn-server/pull/340 (failure is the damn `org.apache.brooklyn.feed.jmx.RebindJmxFeedTest.testJmxFeedIsPersisted` issue, not related) --- If your project is set up for it, you can reply to this email and have

[GitHub] brooklyn-server issue #340: ConfigMap refactor, respecting inheritance

2016-09-22 Thread ahgittin
Github user ahgittin commented on the issue: https://github.com/apache/brooklyn-server/pull/340 @aledsage thanks for detailed review so far; all comments addressed. i know it's a big PR but all the changes are intimately related -- sorry for the pain. hopefully you like what it has

[GitHub] brooklyn-server issue #340: ConfigMap refactor, respecting inheritance

2016-09-21 Thread ahgittin
Github user ahgittin commented on the issue: https://github.com/apache/brooklyn-server/pull/340 I think there are just two things discussed last week which I've not addressed: * For classpath URL resolution from runtime ancestors, the caller still needs to look up the declari