Re: Please review munin-common

[Francois Tigeot <ftigeot%wolfpond.org@localhost>]

Thanks for the review Thomas,

On Sun, Feb 27, 2011 at 03:33:48PM +0100, Thomas Klausner wrote:
> On Tue, Jan 25, 2011 at 07:03:06PM +0100, Francois Tigeot wrote:
> > 
> > munin-common is a supporting package containing the common components 
> > used by munin-master and munin-node. Of the three munin packages 
> > currently in wip/ , it is the oldest and the most complete.
> 
> Please add comments to your patches.

Done.

> The ones for munin-common look partly upstreamable, please try that.

Done. I have opened a ticket and attached the relevant paches in the
Munin trac system.

> The packages should share a Makefile.common since they use the same
> distfile etc.

This is on my todo-list. I hoped to get munin-common cleared and still
be compatible with the old munin-1.3.x packages as a first step.

> bsd.pkg.mk should always be the last to be included (fixed).
> Relative paths to other packages must start with "../../", "../" is
> not sufficient (fixed).

OK.

> I don't think www/cgi-bin is a standard path (is it?) install the
> files perhaps to share/munin/cgi-bin or libexec/cgi-bin.

My munin-master installation is on a FreeBSD machine I have converted
to pkgsrc. I think this path comes from the old FreeBSD port; I'll try
to be more careful to use the pkgsrc defaults in the future.

> Instead of PLIST.${OPSYS}, use PLIST.${OPSYS}, i.e. separate
> PLIST.Linux etc. files.
> Though in this case, where the same files are installed on multiple
> opsys, I would use PLIST.${FEATURE} and set that depending on the
> opsys.

OK. It was a first step cleaning the mess from the old munin-node 1.3.x
pkgsrc package, which uses a long list of PLIST.${OPSYS} and
PLIST.not-{$OPSYS} variables.
I'm well aware munin-master and munin-node are not ready to go in pkgsrc
proper at this time.

I'll try to use a common base as much as possible for the PLIST items.

> The distfile itself isn't fetchable from the master site, please check
> the MASTER_SITES definition.

I have simplified it; it seems to work much better now.

> munin-{master,node} aren't marked as destdir-installable, please
> change that.

I'll do a second pass on them in the future.
 
> That's all for now :)

My plate is full but I hope munin-common is clean this time :o)

-- 
Francois Tigeot

------------------------------------------------------------------------------
Free Software Download: Index, Search & Analyze Logs and other IT data in 
Real-Time with Splunk. Collect, index and harness all the fast moving IT data 
generated by your applications, servers and devices whether physical, virtual
or in the cloud. Deliver compliance at lower cost and gain new business 
insights. http://p.sf.net/sfu/splunk-dev2dev 
_______________________________________________
pkgsrc-wip-review mailing list
pkgsrc-wip-review%lists.sourceforge.net@localhost
https://lists.sourceforge.net/lists/listinfo/pkgsrc-wip-review