Re: [libvirt] [PATCH v2 05/14] nodedev: Use common naming for virnodedeviceobj
On 05/26/2017 03:15 AM, Peter Krempa wrote:
On Thu, May 25, 2017 at 15:57:02 -0400, John Ferlan wrote:
> A virNodeDeviceObjPtr is an @obj
>
> A virNodeDeviceObjListPtr is an @devs
>
> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
> ---
> src/conf/virnodedeviceobj.c | 130 ++++++++++++++++++++++----------------------
> 1 file changed, 65 insertions(+), 65 deletions(-)
>
> diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c
> index 913cdda..a2d09ad 100644
> --- a/src/conf/virnodedeviceobj.c
> +++ b/src/conf/virnodedeviceobj.c
> @@ -34,10 +34,10 @@ VIR_LOG_INIT("conf.virnodedeviceobj");
>
>
> static int
> -virNodeDeviceObjHasCap(const virNodeDeviceObj *dev,
> +virNodeDeviceObjHasCap(const virNodeDeviceObj *obj,
I'm not a fan of this. Adds line churn and it's not really worth it.
Undo-ing would alter so many patches going forward which causes such a
ripple effect w/r/t the process I've been using for all these patches
and a "common" look and feel.
I get the churn and back port thing - really, but I was asked to undo
the single monolithic patch and this is what happens - (too) many
smaller patches each that can be debated and affect the final result.
So, is it technically wrong to rename or is it just not being a fan?
Just because I'm not a fan of something someone else has done, doesn't
make it wrong.
While one could focus on a module by module approach, for me it's been a
bit more global. In some code the vir<driver>Ptr is referenced as an
@obj while other times it's referenced as @<drv>. In the same module
there's vir<driver>ObjPtr's that are sometimes referenced as @obj and
other times as @<drv>. (where <drv> is some shorthand, dev, iface, net,
filter, secret, pool, vol, etc.).
In this module an @device is a virNodeDeviceObjPtr in
virNodeDeviceObjAssignDef and a virNodeDevicePtr in
virNodeDeviceObjListExport. So without going back to the argument
definition how does one really know what it is. If one creates a common
way to describe arguments, then one doesn't have to think about it. So
that's been my approach.
From my perspective of trying to create this "common" look
and feel to
arguments across all these various modules and to help me keep some
amount of sanity, I've tried to think of vir<driver>ObjPtr as @obj and
vir<driver>Ptr as @drv.
Of course there's also the vir<driver>ObjListPtr's that as was pointed
out could be changed into @objs (following the same logic); however, for
me that was too close to @obj, so I left those as devs, nets, nwfilters,
pools, ifaces, etc. (in general).
John