On Thu, 2010-06-17 at 18:05 +0100, [email protected] wrote: > From: marios <[email protected]> > > --- > server/lib/deltacloud/drivers/ec2/ec2_driver.rb | 5 +++-- > .../drivers/rimuhosting/rimuhosting_driver.rb | 7 ++++++- > server/views/instances/show.html.haml | 5 +++++ > server/views/instances/show.xml.haml | 3 +++ > 4 files changed, 17 insertions(+), 3 deletions(-)
Patch looks good in general. There's one thing that I don't like: > diff --git a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb > b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb > index 076bfc4..b49c5e5 100644 > --- a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb > +++ b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb > @@ -22,7 +22,7 @@ require 'AWS' > class Instance > attr_accessor :keyname > attr_accessor :authn_error > - > + attr_accessor :launch_time > def authn_feature_failed? > return true unless authn_error.nil? > end I hadn't noticed that this extension of the Instance class had slipped in. I am very much against doing this, since it will lead to driver-specific models with ugly ripple effects all the way to the views. The Instance class should be declared in one place, with all its attributes. Why can't we check with instance.keyname.nil? and instance.launch_time.nil? if these properties were provided ? David _______________________________________________ deltacloud-devel mailing list [email protected] https://fedorahosted.org/mailman/listinfo/deltacloud-devel
