[Bug 435724] Review Request: fedora-ds-graph - an rrdtool-based graphing utility for Fedora Directory Server

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=435724


Oron Peled <oron at actcom.co.il> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |oron at actcom.co.il




--- Comment #1 from Oron Peled <oron at actcom.co.il>  2008-08-14 16:21:12 EDT ---
This is not an official review, but some preliminary observations:

1. There is no ownership info in %files.
   You should add something like:
%defattr(-,root,root,-)
   Or alternatively specify %attr(-,owner,group) before files
   that have specific ownership/groupship.

2. Release tag should be:
Release: 1%{dist}

3. License should be clarified. Pick the correct string from:
     http://fedoraproject.org/wiki/Licensing
   More info in:
     http://fedoraproject.org/wiki/Packaging/LicensingGuidelines
   Your LICENSE file looks like GPLv2

4. This is a web application and it would fail to run with SELinux
   activated. You should fix the context of the cgi script.
   For a quick howto, look at:
     http://fedoraproject.org/wiki/PackagingDrafts/SELinux
   And probably create a subpackage fedora-ds-graph-selinux

5. No docs whatsoever. As a minimum you can add the COPYING
   and CHANGELOG files. Also...

6. Since you are the upstream author:
   * Adding a README would be even nicer.
   * The common name is ChangeLog and not CHANGELOG.
   * Exclude the .svn from the tarball, either via
     tar command line flags, or by using 'svn export'
     for the tarball.

7. Dependencies:
   * The cgi is in perl and uses some modules in other packages, so:
Requires: perl, rrdtool-perl
   * Time::Local and Symbol are part of perl package itself. OK.

8. The init script (ds-graph) does not follow Fedora common behaviour:
   * Should use daemon function.
   * Should touch and rm /var/lock/subsys/ds-graph
   * Should observe exit status and return appropriate values.

Hope it helps and thank you.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.