[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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 veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]