[Bug 231861] Review Request: cyrus-imapd - high-performance mail server (IMAP, POP3, ...)

[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 report.

Summary: Review Request: cyrus-imapd - high-performance mail server (IMAP, POP3, ...)


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


bugzilla at redhat.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |medium
           Priority|normal                      |medium
            Product|Fedora Extras               |Fedora

lkundrak at redhat.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|nobody at fedoraproject.org    |lkundrak at redhat.com
               Flag|                            |fedora-review?




------- Additional Comments From lkundrak at redhat.com  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.