On Thu, Jan 05, 2012 at 12:11:01AM +0400, Pavel Korovin wrote: Thanks for the update, some comments inline :
> Changes: > - collectd updated to version 5.0.1 > - more plugins in -main package > - several plugins added as MULTI_PACKAGES (-curl_json, -dbi, -python) > - PSEUDO_FLAVORS for building subpackages (e.g. to avoid building > databases/mysql if only snmp plugin is needed) We tried to avoid that. People are supposed to use binary packages, and it adds a shitload of stuff to a Makefile that is already quite complicated. Speaking of that, there are a lot of gratuitous changes in your diff that makes it hard to read, can you keep the Makefile diff _simple_ ? > - CONFIGURE_STYLE changed from "gnu" to "autoconf" > - LIBTOOL changed from run- to build dependency: the port now uses > dlopen/dlsym/dlerror from libc I don't see the benefit of that change. Is the dlopen/dlsym/dlerror patch commited upstream, or at least has a chance for inclusion ? otherwise it makes maintainance harder. > - Graphite Carbon plugin added from the external source > (https://github.com/jssjr/collectd-write_graphite) Nice, but i think it'd be nicer to have it as a 'feature patch', see how it's done for mutt's sidebar patch. Similarly, is this plugin candidate for inclusion upstream ? > - rc.d script for collectd(8) Nice. > Patches descriptions/comments: > - patch-configure.in: I was unable to fix the function check routine > for python and commented it out What's failing in that check ? > - patch-src_collectd_conf_pod: added section for write_carbon plugin > - patch-src_plugin_c: use dlopen/dlsym/dlerror from libc instead of > libtool; borrowed the code from Bruno Prémont > (http://mailman.verplant.org/pipermail/collectd/2011-July/004639.html > for original post) > - patch-src_target_replace_c: allow target_replace plugin to rewrite > the plugin name; maybe it's only specific to my local needs but I > don't see any reason for this restriction. Then add a comment in the patch explaining why it's done. Landry
