Re: [Libvirt-cim] [PATCH 06/10] libxkutil: Use virConnectListAllDomains() to fetch domains
On 03/15/2013 04:20 AM, Wenchao Xia wrote:
δΊ 2013-3-15 6:55, John Ferlan ει:
>
Code seems good, +1. Please reduce version condition macro in function
as much as possible, if there are more functions in same file, I think
two mirrored functions in two condition will make code easier to read
in future:
Readability +1, maintainability -1 - that's the reason I went with
interlaced. I also looked at a few other examples in the code that have
used the LIBVIR_VERSION_NUMBER mechanism and they interlace the code.
For me it's good coding practice.
Another option would be to modify the libvirt.spec.in file and change
the following from:
Requires: libvirt >= 0.9.0
to
Requires: libvirt >= 0.10.2
And the aclocal.m4 file to change the following from:
_pkg_min_version=m4_default([$1], [0.9.0])
to
_pkg_min_version=m4_default([$1], [0.10.2])
Then just remove all the "#else" conditions... Since we're at 0.9.0
already, I'm not sure why the code has the lesser checks. However, I
don't have the "history" to say for 100% the right path to take on this...
John