Code review comment for lp://qastaging/~jason-hobbs/charms/trusty/cinder/remove-volume-group

> On 07/22/2015 08:19 AM, Corey Bryant wrote:
> > Hey Jason, thanks for the patch! I have a couple minor comments below.
> Also can you run 'make lint' and fix the results? The amulet failure above
> might have just been an osci hiccup.
>
> Thanks for the review - I fixed the lint and docstring copy/paste errors
> you pointed out.
>
> >> if vg_found is False and len(new_devices) > 0:
> >> + if overwrite:
> >> + ensure_lvm_volume_group_non_existent(volume_group)
> >
> > I wouldn't block merging on this but it seems like you could get rid of
> ensure_lvm_volume_group_non_existent and just add it's logic to
> remove_lvm_volume_group.
>
> I'd prefer to leave it how it is - I like that 'remove_lvm_volume_group'
> does just one thing - it issues the vgremove command. I also think it'd
> be less clear to always call "remove_lvm_volume_group" even if it's not
> there to begin with.

Fair enough, I'm convinced.
>
> Jason