On Mon, 2012-02-27 at 16:48 +0100, [email protected] wrote:
> From: Michal Fojtik <[email protected]>
> 
> This patch will various HTTP error exceptions to
> client library. The exception classes are then used
> to properly report server/client exceptions and they
> should help handle various HTTP errors.

ACK, with a couple nits:

> Signed-off-by: Michal fojtik <[email protected]>
> ---
>  client/lib/deltacloud.rb         |   72 ++++++++------------
>  client/lib/errors.rb             |  140 
> ++++++++++++++++++++++++++++++++++++++

This is more of a longstanding problem, than something about htis patch:
we are very unclean with the namespace of our client gem; we should
really have everything under lib/deltacloud/ for the client to comply
with Ruby conventions.

>  client/specs/errors_spec.rb      |   59 ++++++++++++++++
>  server/views/errors/500.xml.haml |    2 +
>  server/views/errors/501.xml.haml |   13 +---
>  server/views/errors/502.xml.haml |   13 +---
>  server/views/errors/504.xml.haml |   13 +---

The changes to the error templates for 502 and 504 should really be
folded into patch 1/3; that probably means that the changes to 500 and
501 also need to go into that patch, or into a separate one preceding
1/3.

> diff --git a/client/lib/deltacloud.rb b/client/lib/deltacloud.rb
> index 614eab2..a52af1f 100644
> --- a/client/lib/deltacloud.rb
> +++ b/client/lib/deltacloud.rb

> @@ -349,6 +348,28 @@ module DeltaCloud
>        headers
>      end
>  
> +    def response_successfull?(code)

There's a typo in successful: it should only have one l.

David


Reply via email to