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=458643 Felix Kaechele <felix@xxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED AssignedTo|sayamindu@xxxxxxxxx |felix@xxxxxxxxxx --- Comment #14 from Felix Kaechele <felix@xxxxxxxxxx> 2008-12-17 04:17:21 EDT --- Here are some issues that need to be addressed imo (without trying to impose my personal style of writing specs :-) before I can start the "real" review process - why do you %define real_name DansGuardian when it is never used in the spec any further? - why does BuildRequires: gcc-c++, have a ',' at the end when there's nothing following? - you should better include dansguardian.httpd and dansguardian.init into a source file. That would improve readability of the spec. - you seem not to have written a patch as suggested in comment #8 - in %files why do you set %defattr(-, root, root, 0755) instead of %defattr(-, root, root, -)? Is it really necessary to mark all files executeable? - same for %defattr(0700, nobody, nobody, 0755) a few lines after that - don't put www files in /var/www. You should rather put them into /usr/share/dansguardian. Then you would also need to rewrite the apache config that is supplied. See the package phpMyAdmin for an example. -- 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. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review