Skip to content
This repository has been archived by the owner on Oct 16, 2019. It is now read-only.

add the devices section to the domain specification #9

Merged
8 commits merged into from
Oct 25, 2010

Conversation

calavera
Copy link
Contributor

Hey Mitchell, I just added the devices section to the spec, there are still some missing sections under devices but it's mostly complete.

I'm getting a convention that I think we should follow, any primary element that requires an attribute gets it from the constructor, for instance the disk specification takes the type as mandatory parameter:

<disk type='file'>...</disk>

Libvirt::Spec::Disk.new('file')

I've also seen that we don't have any test on the spec, I'll try to add some basic ones this weekend.

@mitchellh
Copy link
Owner

David, I'm currently out of town so I'll look at this when I get a chance :) It looks good its just a lot to grasp and such.

As for the tests, yes some tests would be good. I initially didn't do any testing since most of the tests would be trivial and not very helpful. "assert XML has memory in the output" and so on :P But I think testing to make sure the more complex logic that is visible works.

@calavera
Copy link
Contributor Author

No problem, so I have time to add the tests :) Enjoy your weekend

@mitchellh
Copy link
Owner

Back from the weekend! Looks really awesome! Amazing. I'm working on getting this in now.

@mitchellh
Copy link
Owner

I do notice a couple things I'm going to switch around in the coming days but it mostly looks good. Great job.

This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants