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=472098 --- Comment #3 from Andreas Thienemann <andreas@xxxxxxxxx> 2008-11-20 09:35:56 EDT --- David mentioned you'll be needing someone sponsering you. I'll therefore continue with the review. About the dependencies: RPM as used by fedora currently does not offer soft dependencies. Either they are must be fulfilled or they are not needed. I'm usually leaning towards including them as hard dependencies for the users sake. Not everyone reads manuals. :) Some notes about the spec: Wrapping lines at 78chars makes it much more readable. For Requires or BuildRequires lines, the tag can be repeated several times. For the sake of readability: It might be worth considering different .spec files for fedora based and opensuse based distros, the %if/%endif conditionals make reading the spec a bit harder. If you want to keep the single .spec file, at least consider using only one %if %ifelse %endif block and execute all needed actions inside of it instead of repeating the %if %endif construct for a single command. The mkdir -p/cp combo used in the %install phase can be abbreviated as "install -D -m perms source dest". This way the directories leading up to it are created automatically and usually the install part can be written much more condensed. The bigger blocker is IMHO the %post script. Most of the commands it executes are unnecessary and can be implemented during the %install phase. e.g. mkdirs are better done during the %install phase and packaged in the %file section. That way these files are included in the rpmdb and are linked to a package. chmod commands are equally best done during the %install phase. %attr macros can be used in the %file part of the spec to assign special permissions to single files. Calling mozroots to change a local certificate store is a bad idea IMHO. This way possibly existing local policies are circumvented in a install script. Starting mysqld is a bad idea as well. It might be that the real mysqld is on a different machine and the local mysqld is therefore not needed. The dependency on it is acceptable IMHO, but starting it specifically is not. Furthermore, what if the mysqld init script is disabled? Just starting it once will not survice a reboot. Updating the database schema might be acceptable but the general suggestion is to not execute any SQL commands during a %post script but leave it to the system admin in question. They know their system much better then any packager can ever do. David's complaint about the %post script echoing messages is perfectly valid. RPM installations but especially upgrades might be performed at night and fully automated. Nobody is going to see these messages. The concept of rpm is unattended installation and this design principle should not be violated. Can the different %{webroot}/dekiwiki/bin %{webroot}/dekiwiki/editor etc. lines in the %file-section be condensed into one single %{webroot}/dekiwiki/ line? I'll go about building and installing the software later today for detailed review. So much for my first notes about the .spec file. -- 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