On Fri, Aug 24, 2012 at 09:49:38AM -0600, Eric Blake wrote: > 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. Oh, sure push :-) Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list