remove general include of REXML

[Xylakant]

this fixes #31 where the provider fails on CentOS 6.5 (probably on older ruby versions in general)

[igalic]

would be pretty cool if we head beaker tests for this module ;)

[Xylakant]

Probably, but I'm sorry - I can't build those. I can just say that I tested it on CentOS 6.5 and it works for me and it's the only instantiation of a REXML class in the whole file. There's no conceivable reason it should not work on any other ruby either.

[igalic]

I applaud your optimism :|

[igalic]

I'll submit a pr for beaker testing, but I just realized we don't have any libvirt_pool tests :(

[Xylakant]

I agree that tests are always better, but in this case I'll stick with my optimism. There's no other file including the changed file and the type itself doesn't use REXML at all.

I'll give a little bit of ruby education here: You probably never want to use include with any module (REXML is a module) on a global scope. This does funky things, most notably it will include all constants of that module in Object (and thus in all other classes)

[root@app01 ~]# irb
irb(main):001:0> Version
NameError: uninitialized constant Version
    from (irb):1
    from :0
irb(main):002:0> Object::Version
NameError: uninitialized constant Version
    from (irb):2
    from :0
irb(main):003:0>  require 'rexml/document'
=> true
irb(main):004:0> include REXML
=> Object
irb(main):005:0> Version
=> "3.1.7.3"
irb(main):006:0> Object::Version
=> "3.1.7.3"
irb(main):007:0> REXML::Version
=> "3.1.7.3"
irb(main):008:0> class Foo; end
=> nil
irb(main):009:0> Foo::Version
=> "3.1.7.3"

If some other part of the code tries to use a constant named Version (or any of the constants that REXML defines in its scope) you'll get weird error messages that are hard to debug.

[Xylakant]

Actually, maybe this code would work (not tested, depends on the puppet implementation of .provide):

 Puppet::Type.type(:libvirt_pool).provide(:virsh) do
    include REXML
 ...
 end

But the initially proposed change is "nicer" ruby since it avoids including REXML completely.

[Xylakant]

You'll probably want to do the same change in #25 though :)

[igalic]

I'm confuzzled. I thought ruby modules always have global scope.

[igalic]

ack, thanks. Re-reading this, I think I get it now.

[Xylakant]

ok :) Feel free to ask via PM or email if you have ruby questions, since probably this is probably not the right place. If you're interested I'll go over the ruby code and do some style criticism, but I'd move that to a different PR.

[igalic]

I think that criticism should be moved over to #25 ;)
That design is far from done…

[thias]

Thanks! Applying many changes... re-created as 7d3e0bd.