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: cyrus-imapd - high-performance mail server (IMAP, POP3, ...) https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=231861 bugzilla@xxxxxxxxxx changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |medium Priority|normal |medium Product|Fedora Extras |Fedora lkundrak@xxxxxxxxxx changed: What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|nobody@xxxxxxxxxxxxxxxxx |lkundrak@xxxxxxxxxx Flag| |fedora-review? ------- Additional Comments From lkundrak@xxxxxxxxxx 2007-06-28 14:46 EST ------- I'll take this for the review. The package is in Fedora already and proved that it is in solid and usable state. Just a formal review and few notes follow: * specfile and the package properly named * BSD license matches reality and is legally ok * specfile is legible (to some extent :) and text is written in american english * sources match upstream: ac03b02c1ae08d52f807b58c488b204f cyrus-imapd-2.3.8.tar.gz 8f7a26b0556369827bb5c8084a3e3ea1 cyrus_sharedbackup-0.1.tar.gz * builds supported architectures * dependencies list seems to be fine * no locales * no shared libraries * not relocatable * %files section is all ok, so is the ownership of directories ! The %clean section contains old construct * couldn't find an instance of inconsistent use of macros * package is program code ! relatively large amount of documentation is not split into a subpackage * header files and static libraries are in -devel subpackage ! static libraries are present * no pkgconfig stuff, nor anything else that would go into a -devel subpackage * devel does not depend on base, but that's okay since it contains no shared libraries, just headers (see below regarding the static libs). * no libtool archives * not a gui application ! the subtle problem with %clean also applies to %install ----- 1.) In %clean section, please remove the test from [ "%{buildroot}" != "/" ] && %{__rm} -rf %{buildroot} it is not needed, as you set the build root here: BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root The same applies for %install section. 2.) Please consider spliting of the html documentation into the -doc subpackage, though it is largely dependent on your personal appeal. Also, I think the html versions of the manual pages should not be packaged as they are redundant -- please remove them. 3.) Please try to avoid static libraries. Does anything use these? ./usr/lib64/libcyrus.a ./usr/lib64/libcyrus_min.a 4.) From the rpmlint result (the whole dump is attached): W: cyrus-imapd conffile-without-noreplace-flag /etc/cron.daily/cyrus-imapd W: cyrus-imapd conffile-without-noreplace-flag /etc/logrotate.d/cyrus-imapd W: cyrus-imapd conffile-without-noreplace-flag /etc/rc.d/init.d/cyrus-imapd Tomas, please fix this -- those should not be marked as config files. Though I believe other rpmlint warnings to be relatively harmless, please at least skim through them. You might well want to silence some of those (as some are trivially easy to fix.) Also, sed might be better suited for what you do with perl in %post. -- 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-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review