Github user aledsage commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/542#issuecomment-78255699
Looks really good! A few minor comments to be addressed, but very close to
being merged.
FYI I did a tiny bit of this in
https://github.com/apache/incubator-brooklyn/pull/547 (see commits
https://github.com/aledsage/incubator-brooklyn/commit/fe5ecca123525b4d6c8cd9f5c0049e3d5b6b12b6
"BrooklynNode: only unmanage if VM terminating",
https://github.com/aledsage/incubator-brooklyn/commit/f736dfc84f79d3aa6275747ad3bdf6e4d9181a0d
"Add SoftwareProcess.preRestart (for BrooklynNode)", and
https://github.com/aledsage/incubator-brooklyn/commit/d38b652519cf4b46632c1b8dd06c52498eb3cdd3
"BrooklynCluster: add memberSpec + test").
I prefer your impl of `preRestartCustom` (that you included the method on
the superclass as well, and that you also added `postRestart` for symmetry.
The test I added is useful though. I can look at that again once this is
merged and I rebase my PR, or you can pull in my test if you fancy.
---
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.
---
