Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Review Request: webmin https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=184080 ------- Additional Comments From kevin@xxxxxxxxx 2006-04-08 18:26 EST ------- Here's A prelim review: MUST items: See below - rpmlint output OK - Package name. OK - Spec file name matches. See below - Package guidelines. See below - License. See below - License field matches in spec. See below - License included in files. OK - Spec in american english. OK - md5sum of source from upstream c45fe387902405cb36a1a5c6a240ad0d webmin-1.260.tar.gz c45fe387902405cb36a1a5c6a240ad0d webmin-1.260.tar.gz.1 OK - Compiles and builds on one arch at least. OK - No forbidden buildrequires included See below - Owns all directories it creates. OK - No duplicate files in %files listing. See below - Permissions on files correct. See below - Clean section correct. OK - Macros consistant. OK - Code not content. OK - Doesn't own any files/dirs that are already owned by other packages. Items needing attention: Given the number of issues here, (and I didn't even finish looking through everything), perhaps it would be better to start off with a clean spec generated from 'fedora-newrpmspec -t perl -o webmin.spec' and build from there? Let me know how you want to move forward. 1. Don't use 'fe5' in release, instead use %{?dist} tag. http://fedoraproject.org/wiki/DistTag 2. Don't include a 'Vendor' tag. It gets set by the build system. 3. You shouldn't need the [ "%{buildroot}" != "/" ] in the %clean section. 4. You shouldn't "echo "Running uninstall scripts .."" in the un-install. rpm shouldn't output anything to stdout. There are several other places with echos that should be removed. 5. You copy the /etc/webmin dir to a backup, is that likely to be large? rpm can't account for that space when it says ok to the install, so if the disk is close to full, wouldn't that cause a nasty failure? 6. version 1.270 is now out. 7. so the LICENSE file is BSD, but it looks like several of the modules have GPL headers on them. Doesn't that mean the entire package has to be distributed under the GPL? 8. There is quite a lot of rpmlint output. (around 250 lines) available on request. 9. You create/remove the /var/webmin dir in post/postun/triggers. This is not acceptable. You must include it in files. Does this have log files and other state? You shouldn't delete it on package removal. 10. The run-setup.sh script needs a LOT of work. It runs perl commands on stdin? It checks for about 100 diffrent types of OSes, which this package will know is fedora. It ends up just running /usr/libexec/webmin/setup.sh with some env set. Perhaps you could re-write it as a clean/simple perl/shell script? 11. Why are you doing "%define __spec_install_post %{nil}" ? 12. Why 'mkdir -p %{buildroot}/etc/rc.d/{rc0.d,rc1.d,rc2.d,rc3.d,rc5.d,rc6.d}' ? You don't install anything there, and chkconfig should handle that on install. 13. /usr/libexec/webmin/run-uninstalls.pl is totally unacceptable. It asks if the user wants to un-install webmin and then runs 'rpm -e --nodeps webmin'This would be executed from a rpm %preun and totally fail in a lot of ways. -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug, or are watching the QA contact. -- fedora-extras-list mailing list fedora-extras-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/fedora-extras-list
