On Mar 7, 2013, at 8:10 AM, mfoj...@redhat.com wrote:
> Good news everyone!
>
> I started a complete rewrite of the deltacloud-client component
> of Deltacloud core repo few week ago, because:
>
> 1. Too much *magic* in the current client (meta-programming, etc)
> 2. No documentation (see 1))
> 3. Hard to navigate in code and fix bugs (see 1,2)
> 4. Hard to add collection that does not follow the common 'schema'
> 5. Error reporting sux (even we have superb error API in DC-core)
>
> And since I keep hearing complaints mainly about 5) and 3), I decided
> to change it.
>
> This brand new client is build using the Faraday[1] library, which do a
> better job in handling errors (using middleware[2]) and there is no magic
> needed to get it handle basic HTTP auth and HTTP headers properly.
>
> Faraday has also built-in support for VCR (or better VCR has built-in
> 'hook into faraday'), so working with VCR is a pleasure ;-)
>
> The code structure of new client is following:
>
> lib
> - deltacloud
> - client
> - models
> - methods
>
> The *methods* contains methods ;-) used to communication with Deltacloud API.
> So for example 'create_instance', 'instances', 'images', etc.
> While the *models* contains the XML deserialization part and some models also
> include *method* file, so you can write something like:
>
> inst = client.instance('foo')
> inst.reboot! # => calls client.reboot_instance(@self)
>
> Part of this exercise was to keep the codeclimate[3] ratings high, so in other
> words avoid duplication and complexity :-)
>
> !!! IMPORTANT NOTE !!!
>
> This patch *might* break existing apps that use the old client. I tried really
> hard[4] to keep all methods from previous client, but there might be some
> explosions. Please let me know if you find some :-)
>
> For example:
>
> Deltacloud::Client(url, user, pass) instead of DeltaCloud.new(url, user, pass)
>
> Some patches might not be sent properly (especially 4/5 with VCR fixtures).
> For that please refer to tracker: http://tracker.deltacloud.org/set/374
>
> Cheers,
> Michal
>
> [1] https://github.com/lostisland/faraday
> [2] https://github.com/lostisland/faraday#writing-middleware
> [3] https://codeclimate.com/github/mifo/deltacloud-client
> [4]
> https://github.com/mifo/deltacloud-client/blob/master/lib/deltacloud/client/methods/backward_compatiblity.rb
>
Michal,
This looks great. Many of my comments are really questions regarding
why you used Ruby the way you did.
I've formated my feedback/questions in the attached patch.
I had initially started making code changes but then decided to
just leave comments/questions in the code, prefaced wtih "Joe".
I've also done some tpyo and grammar fixes and comment changes to
attempt to make the comments more consistent.
A general question: Are we trying to stick to 80char line lenght?
Hope this helps!
Joe V.
