Hi,
This patchset should improve the code we have currently for CIMI.
I will briefly explain what each patch did and why:
1/8:
This is just cosmetic change. It's better to have consistent DSL
rather than using 'global!' method inside all collections.
If some collection want be 'global' lets call it 'global_collection'.
This change was tested on Ruby 1.8 and 1.9 and should not break anything.
2/8:
Using EOS is a bit weird to me. We used EOL just because our Deltacloud
descriptions was too long to fit into one line ;-)
However the CIMI descriptions are short one liners so why not put them
into regular string.
3-6/8:
The code in cmwgapp_helper was too much complex to me to undestand
(and read) so I decided to rewrite it. You can now see the difference.
I dunno how about you, but I had very hard time to undestand what
'fixup_content' or 'respond_to_collection' methods internally do.
We can discuss it, but my personal opinion is that hacking XML using
XmlSimple and then modifying resulting Hash is too complex.
7/8:
The code previosly used to return collection was using XmlSimple
and not HAML. This throw an Exception in Ruby 1.8:
NoMethodError - undefined method `downcase' for 77:Fixnum:
./bin/../lib/cimi/helpers/cmwgapp_helper.rb:68:in `respond_to_collection'
This is one of the examples of darkside XmlSimple. The code is too
complex and thus there is higher propability of exceptions like this one.
I spend 2 hours debugging the code until I finally discover from where
this problem original came.
The 'index.xml'haml' files are easy readable, we can update them according
to CIMI everytime and everyone can undestand what should be returned on output
just by looking on HAML file. Later on, we can re-use this index.xml.haml
as we did in Deltacloud and use 'partial' rendering to render items in
collection.
8/8:
I had long conversation about this with Tong Li and as far I undestand
his point of using this variable is to make a bridge between Deltacloud API
object model and CIMI. However I think we can easely change our object
to support CIMI specific properties very easely without breaking anything.
Since every class is open we can simple 're-open' the model class (like
Instance) and add more methods here.
My personal opinion is that as this implementation will grow and we will
need to add more data and properties, this bridge will become our nightmare.
-----
IMPORTANT: This patch is not addressing the UI. The HAML views will need to
be updated to support new local variables and stuff.
Especially 'new' operation views, will need to be updated together
with our models. I'll take this task once we will have basic
Test::Unit ready (to be sure I'll not break anything).
PS: This patchset was tested under Ruby 1.8 and 1.9 (and Rubinius for fun ;-).
-- Michal
