On 3/7/19 9:26 AM, John Ferlan wrote:

>>> +    VIR_SNAP_STATE_PMSUSPENDED = VIR_DOMAIN_PMSUSPENDED,
>>> +    /* Additional enum values local to qemu */
>>
>> As Eric Blake pointed out in an earlier iteration:
>> s/qemu/snapshots/

D'oh - it's a bad sign when I miss incorporating my own review comments.


>>>
>>> /* virDomainSnapshotState is really virDomainState plus one extra
>>> state */
>>> -VIR_ENUM_IMPL(virDomainSnapshotState, VIR_DOMAIN_SNAPSHOT_STATE_LAST,
>>> +VIR_ENUM_IMPL(virDomainSnapshotState, VIR_SNAP_STATE_LAST,
>>
>> VIR_DOMAIN_SNAPSHOT_STATE would be the least surprising prefix
>>

Indeed, but it is also longer, and runs into line-wrap issues (I'm not
opposed to it, though).

> 
> <sigh>, true, but we could go with virSnapState to ensure we match the
> typedef nomenclature with the enum symbol names or change the _SNAP_ to
> _SNAPSHOT_ w/ virSnapshotState for the typedef - whatever is easiest.

I concur with having consistency one way or another. Unless I hear a
strong preference, I'll go ahead and spell the values out in full (and
wrap lines) rather than trying to abbreviate the values but not the enum
name.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to