Re: [libvirt] [PATCHv2 3/3] snapshot: rename an enum

On 08/24/2012 01:00 AM, Daniel Veillard wrote: > On Thu, Aug 23, 2012 at 02:54:16PM -0600, Eric Blake wrote: >> The name 'virDomainDiskSnapshot' didn't fit in with our normal >> conventions of using a prefix hinting that it is related to a >> virDomainSnapshotPtr. Also, a future patch will reuse the >> enum for declaring where the VM memory is stored. >> >> @@ -11199,7 +11200,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, >> goto cleanup; >> } >> if (virDomainSnapshotAlignDisks(def, >> - VIR_DOMAIN_DISK_SNAPSHOT_EXTERNAL, >> + VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL, >> false) < 0) >> goto cleanup; >> if (qemuDomainSnapshotDiskPrepare(vm, def, &flags) < 0) > > ACK, the only annoyance is that the name being longuer the simple > substitution leads to lines over 80 characters, I would be a proponent > of dropping the indentation to the opening ( for the sake of keeping > everything on one 80 char line Compared to the original: VIR_DOMAIN_DISK_SNAPSHOT_EXTERNAL virDomainDiskSnapshotTypeFromString I thought of various other possibilities for shorter names, such as: VIR_DOMAIN_SNAPSHOT_STYLE_EXTERNAL virDomainSnapshotStyleTypeFromString VIR_DOMAIN_SNAPSHOT_LOC_EXTERNAL virDomainSnapshotLocTypeFromString VIR_DOMAIN_SNAPSHOT_LOCATION_EXT virDomainSnapshotLocationTypeFromString but I'm not sure that I like the abbreviation 'loc'. Should I go ahead and push the patch as-is, or go with one of the shorter names? After all, long lines are a cosmetic issue, and I'd rather code with long names that are legible than short names that leave me scratching my head.