On Wed, Jul 26, 2017 at 10:57:30AM -0400, John Ferlan wrote: > > > On 07/24/2017 07:49 AM, Pavel Hrdina wrote: > > On Fri, May 19, 2017 at 09:03:23AM -0400, John Ferlan wrote: > >> Use the structure names in the @data setup - makes it easier that going > >> back to find the struct. > >> > >> Use the @maxnames instead of @nnames since that's what it is. > > > > Please use camelCase -> @maxNames. > > > > This I disagree with as maxnames is used *liberally* throughout many > places in libvirt and in particular as arguments to functions. In > particular, follow this back to : > > > virDrvConnectListNetworks > virDrvConnectListDefinedNetworks > > and > > virConnectListNetworks > virConnectListDefinedNetworks > > which all use @maxnames. > > But I will separate and describe appropriately. On the other hand there is for example maxMemory which uses camelCase. I have to disagree with your disagreement :) but argument that something is used in a certain way is not a good argument because the old usage may be wrong. > > >> > >> Modify the @filter to be @aclfilter and change the typedef from > >> virNetworkObjListFilter to virNetworkObjListACLFilter. > > > > NACK to this change, even though it's used only to filter by ACLs, it > > can be used to filter by anything. > > Again, I disagree. I've been using @aclfilter in other drivers and > taking this route makes things consistent. Now that I've checked it, even virDomainObjListExport has the type with ACL in it. So I take my NACK back :). Anyway, for the name I would prefer to use aclFilter but since the ship has sailed I give up. It can be done as a followup cleanup. But please keep this in mind that the camleCase is preferred. Having no separation between words in the variable name reduces readability. Pavel > Besides, look at a few of the vir*ObjListExport* type functions where > there's actually a second filter that would take an @obj and a @flags > argument and could be defined "generically" as "@filter". Now if there > was a "generic" ObjListExport routine, that @filter could be an element > to a common structure too... > > I will though separate it out. > > Tks - > > John > > > > > This patch does three things in one, so it should be three separate > > patches. Since the last change is not correct split the remaining > > changes into two patches. > > > > Pavel > >
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
