----- Original Message -----
> [...]
> commit a3c75c74e4e391ec36022468b8aa839c1e4a2693
> Author: Frank Ch. Eigler <fche@xxxxxxxxxx>
> Date: Tue Dec 1 09:52:05 2015 -0500
>
> pmfg docs: mention PR1129
>
> Since pmDupContext is not really functional (PR1129), adjust the
> fetchgroup documentation [...]
Isn't the fix for http://oss.sgi.com/bugzilla/show_bug.cgi?id=1129
... just to do a deep copy of the attributes? (i.e. walk the hash,
copy the attrs from the original context into the dup)
That seems too easy - surely you'd have simply fixed that and not
bothered with bugzilla, extra documentation specially referring to
the bug - what am I missing there?
Haven't reviewed the rest of the series, but one probable issue from
your description is that the pmclient tool is a demo program that was
written to accompany content in the Programmers Guide and man pages -
rewriting it means there's probably going to be a disconnect with the
content in the programmers guide now. (Hmm, and this new API should
probably be added to the programmers guide too, I guess.)
Another thing to think about - if the only use cases so far are pmmgr
and pmclient (both of which are relatively infrequently used, in terms
of the installed base), I'd like to see a few more uses to validate the
new API (esp. since its proposed for libpcp, and not a separate library
as was done for PMDAs and other things). pmstat(1) would seem a good
candidate, and the python APIs of course should expose and use it. It
might even be helpful to look into libpcp_qmc making use of it too?, or
maybe pmwebd?
> [...] I would be comfortable with getting the code merged.
Given it is an API+ABI and there have been several attempts at this one
in the past, there is no rush on merging it. Extra time up-front, and
converting as many places as possible (as was done in the pmGetOptions
situation for example) will help to build up confidence amongst the PCP
maintainers that the API suits as many tools as possible ... so please
add several more cases, and lets tentatively pencil this new feature in
for pcp-3.11.0.
That little pmDupContext crash fix would be welcome anytime, of course.
thanks.
--
Nathan
|