flush out logical volume support

[movitto]

No description provided.

[coveralls]

Coverage decreased (-0%) when pulling 6a922f0 on movitto:lvm-support1 into 42d0486 on ManageIQ:master.

[Fryguy]

As this list grows, would it help to alphabetize these, or does it make more sense to keep them in logical groups (i.e. partitioning, mounting, power, volumes, etc.) If logical grouping is better, perhaps comments before each group might help keep the list clear?

[jrafanie]

@movitto This is still failing on travis, does this pass locally for you? https://travis-ci.org/ManageIQ/linux_admin/builds/9190071

[chessbyte]

Why are there 3 classes (LogicalVolume, PhysicalVolume, VolumeGroup) inside lib/linux_admin/logical_volume.rb? Why not split them out into 3 separate files?

[Fryguy]

@chessbyte Oh yeah, I didn't notice that. Totally agree with the one-class-per-file thing.

[jrafanie]

@movitto I just noticed we're subclassing in many places... are we doing that for the methods from Common? If so, can't we just use include/extend to gain that behavior?

[movitto]

As this list grows, would it help to alphabetize these....

Whatever works. Can be easily adjusted in a subsequent formatting patch.

Minor whitespace here (vg is way to the right).

Done

This is still failing on travis, does this pass locally for you?

I was passing locally, just forgot to take the different environment on travis into account. Hopefully fixed it in the followup.

Why not split them out into 3 separate files?

Done

can't we just use include/extend to gain that behavior?

Ya we should resolve this once and for all to make it consistent across the board. In general, unless there is reason not too, I prefer including modules as mixins, providing methods on instances w/out disrupting the static class hierarchy. If this sounds reasonable to everyone, it can be implemented in a separate reorg patch.

[jrafanie]

@movitto Can you rebase and re-push, it can't be automatically merged?

I opened issue #19 to move from inheritance to composition where we only need LinuxAdmin::Common.

[Fryguy]

The LinuxAdmin::Common was written as subclassing initially because it was complicated to use the mixin when everything was class methods. Now that everything is moving to objects, technically it shouldn't even be a mixin, it should just be another object...in this case, like a utility object. Then when you want to run something you would say something like Common.run or Utils.run or something.

[movitto]

Rebased

[coveralls]

Coverage remained the same when pulling b0b0545 on movitto:lvm-support1 into 28d869d on ManageIQ:master.

[coveralls]

Coverage remained the same when pulling b0b0545 on movitto:lvm-support1 into 28d869d on ManageIQ:master.