Thanks for the contribution @igreenfield! I've just done an initial review of the code (I've not dived into the details yet) and here is an initial checklist to address:
**Code style** * [ ] Use a 2 indent for XML files. * [ ] Use a 3 space indent and 120 line length for Java files. * [ ] Run `mvn checkstyle:checkstyle` and address all violations. You'll find them in the `target/checkstyle-results.xml` file. **Code** * [ ] Change all waits (all while + Thread.sleep combinations) to use the jclouds `Predicates2.retry` helper. * [ ] All constructors must use the `Preconditions.checkNotNull` helper from Guava to validate that all mandatory fields are present. * [ ] Remove all company references in javadocs. * [ ] In jclouds we avoid using `null` (see [why](https://code.google.com/p/guava-libraries/wiki/UsingAndAvoidingNullExplained)). Public methods in classes and interfaces that could return a `null` should be changed to return an `Optional` instead. **Tests** * [ ] There is no single test (the two only classes in the tests folder have the tests disabled). Every function, predicate, supplier, etc (all added classes) **must have the corresponding unit and live tests**. This is **critical** to have the pull request merged, otherwise we can't know if the provider is working as expected or not, and even worse, we won't be able to evolve, change, and maintain it, as we won't know if we're breaking the existing stuff. These are some basic things that should be addressed before getting this merged and this is quite a large pull request, so by nature will take some time to review. Please, be patient. I look forward to seeing it progress! Once the changes in the checklist are complete we'll be in a better position for a second round review, and go through the functional and implementation details. Thanks for your work here @igreenfield! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/61#issuecomment-47100656
