[libvirt] [PATCH 15/16] network: Modify naming for virNetworkObjList* fetching APIs
Pavel Hrdina
phrdina at redhat.com
Wed Jul 26 15:40:44 UTC 2017
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
> >
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170726/c19c34e9/attachment-0001.sig>
More information about the libvir-list
mailing list