ACK, couple thoughts: On 10/02/12 14:56, [email protected] wrote: > From: Michal Fojtik <[email protected]> > > > Signed-off-by: Michal fojtik <[email protected]> > --- > server/lib/deltacloud/base_driver/exceptions.rb | 16 +++++++ > server/lib/deltacloud/drivers/mock/mock_driver.rb | 32 +++++++++++++- > .../lib/deltacloud/helpers/application_helper.rb | 2 +- > server/lib/sinatra/lazy_auth.rb | 2 +- > server/views/errors/501.html.haml | 43 > ++++++++++++++++++++ > server/views/errors/501.xml.haml | 12 +++++ > server/views/errors/502.xml.haml | 13 ++++-- > server/views/errors/504.html.haml | 43 > ++++++++++++++++++++ > server/views/errors/504.xml.haml | 12 +++++
we really need all of these? like 501.html.haml 504.html.haml are identical, as are 501.xml.haml 502.xml.haml and 504.xml.haml. If we really want seperate files for 'neatness' perhaps just include them in each other? just thinking in terms of maintaining if we want to change something its gonna be messy. One more thought below: > diff --git a/server/lib/deltacloud/base_driver/exceptions.rb > b/server/lib/deltacloud/base_driver/exceptions.rb > index e44c89b..9effe29 100644 > --- a/server/lib/deltacloud/base_driver/exceptions.rb > > def realms(credentials, opts=nil) > - return REALMS if ( opts.nil? ) > - results = REALMS > + check_credentials( credentials ) > + results = [] > + safely do > + # This hack is used to test if client capture exceptions correctly > + # To raise an exception do GET /api/realms/50[0-2] > + raise "DeltacloudErrorTest" if opts and opts[:id] == "500" > + raise "NotImplementedTest" if opts and opts[:id] == "501" > + raise "ProviderErrorTest" if opts and opts[:id] == "502" > + raise "ProviderTimeoutTest" if opts and opts[:id] == "504" > + return REALMS if ( opts.nil? ) > + results = REALMS > + end > results = filter_on( results, :id, opts ) > results you want to keep this stuff here? I saw that you are using them in client/specs/error_spec but feels a bit too 'hacky' - maybe define some special method just for this? marios
