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