Re: [libvirt] [PATCH 1/2] Introduce virDomainYesNo enum type

>>> } >>> - def->os.bios.useserial = VIR_DOMAIN_BIOS_USESERIAL_YES; >>> + def->os.bios.useserial = VIR_DOMAIN_YES_NO_ENABLED; >>> } else { >>> - def->os.bios.useserial = VIR_DOMAIN_BIOS_USESERIAL_NO; >>> + def->os.bios.useserial = VIR_DOMAIN_YES_NO_DISABLED; >>> }

>>> if (def->data.spice.filetransfer) >>> virBufferAsprintf(buf, "<filetransfer enable='%s'/>\n", >>> - virDomainGraphicsSpiceAgentFileTransferTypeToString(def->data.spice.filetransfer)); >>> + virDomainYesNoTypeToString(def->data.spice.filetransfer)); >>> } >> >> I'm not really a fan of this cleanup, as IMHO the result is less clear & >> harder to follow than the original code. > > How so? The original code was very repetitive, with multiple enums (all > with long names) copying the same few enum elements. We're not painting > ourselves into a corner - if any of the replaced enums ever grows a > third value (such as "on", "hybrid", "off"), then we just break that one > enum back into a named list rather than using the generic on/off enum. > I'm actually in favor of this cleanup. Specifically a enum constant name like YES_NO_DISABLED is just awful IMHO compared to the original desriptive name.