Thanks for the cleanup @luciano-sabenca-movile!

I've checked out the code and now I understand those base classes. They are 
reused in the strategies that list all entities, and in strategies that list 
only the entities in one environment. My concerns were because I thought there 
were only used by one class, thus my comments about the premature optimization. 
Other strategies don't have the logic in an abstract base class, so it is OK.

There are a couple things left before merging the PR:

* [ ] The `ListEnvironment` strategy still declares a 
`ListeningExecutorService` in the interface. Could you change it to a standard 
`ExecutorService`?
* [ ] Run a `mvn checkstyle:checkstyle` and fix the checkstyle violations. 
You'll find them in the `target/checkstyle-result.xml` output file.

The PR looks good, and once these small bits are fixed I'll merge it. Thanks!

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-chef/pull/47#issuecomment-49415150

Reply via email to