Github user aledsage commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/1045#issuecomment-159071176
Thanks @hzbarcea
The `JcloudsLocation` code needs a complete re-write. It's too big, too
many if statements, too much cloud-specific code in the middle of methods (e.g.
if aws, or if docker), not unit testable, a fairly ad hoc set of injection
points for customising the provisioning behaviour, etc. I also agree about the
way `if windows` being a big code smell.
I think that at some level of our code, we'll hit the use of an ssh-client
or a winrm-client (unless we went all-in with chef/puppet/salt etc). Therefore
I think the bulk of this PR is not related to the problems you refer to. We
want to remove the dependency on `io.cloudsoft.winrm4j` that had leaked into
too many parts of the code.
As for `WinRmMachineLocation`, I think this code improves it by putting the
right (or at least more right) responsibilities into the winrm-client.
I'd therefore like us to merge it before we tackle the big
`JcloudsLocation` and other such classes.
---
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.
---
