On 05/21/2013 06:41 PM, Nathan Scott wrote:
Looking good - couple of minor nits ...
pmda.h - this is an ABI breaking change, doesn't have to be if you move the
new enum entry to the end of the set rather than in the middle.
OK. Looks like the patch you sent me incorporates this.
open.c - this too doesn't really need to risk fallout from changing the API.
Instead, I'd suggest keeping __pmdaOpenInet, add __pmdaOpenIPv6 and make 'em
one-line wrappers around your new __pmdaOpenIP (modified __pmdaOpenInet). I
wonder if __pmdaOpenSocket might be a more natural fit as the new name here,
rather than __pmdaOpenIP?
This is not really an API change, since __pmdaOpenInet() was a static
function. I have no issue with this suggestion, however, and it is
consistent with the changes you made to dbpmda (also in your patch), as
is the suggested name __pmdaOpenSocket(). I will make the change.
Some guidance regarding which qa test(s) to augment would be
appreciated. It's hard to tell by number :-(
110 156 199 seem like initial candidates. Looks like they'll need the pmcd
and pmdaproc.sh support in place first though.
Thanks. I'll have a look.
Dave
|