[Bug 231861] Review Request: cyrus-imapd - high-performance mail server (IMAP, POP3, ...)
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=231861 Michal Hlavinka mhlav...@redhat.com changed: What|Removed |Added Flag|fedora-review? | --- Comment #14 from Michal Hlavinka mhlav...@redhat.com 2009-10-30 10:33:51 EDT --- fixed, that flag was set by previous reviewer whose account has been closed -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 231861] Review Request: cyrus-imapd - high-performance mail server (IMAP, POP3, ...)
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=231861
--- Comment #11 from Michal Hlavinka mhlav...@redhat.com 2009-10-29 11:49:54
EDT ---
(In reply to comment #10)
Is this package still ready for review?
somehow :o)
(And shouldn't it be a Merge Review?)
definitely
How can it be that it has status Assigned but isn't assigned to anybody?
afaict, it seems it was assigned to someone whose account has been closed
My first impression is that it looks like it haven't been dressed up for examn
and could use some polishing before a final review.
sure, I've inherited this package and it required a *lot* of polishing. But
after a few rounds it got lower and lower in my todo-list, especially because
there was no reviewer.
Some brief comments:
It seems like most (all?) of the code now is licensed announcement-BSD-ish, so
the License and the comments about it are a bit misleading.
fixed
The spec is quite complex and verbose and IMHO not easy to read.
I agree
The spec contains comments left over from the Invoca version.
fixed
_perlhack variable seems to be unused since 7.3 - Red Hat, not Fedora!
yes, this was leftover, I've removed all perlhack ifdefs some time ago
removed
There are manu variables and configuration options. Are they necessary and
used?
this is what I can't even guess actually. I'd definitely like to get rid of all
those switches, but I don't want to break it for someone... Well, I've just
though about removing them in new rawhide and wait if someone complains.
and since devel is future rawhide now, I've removed them
The %file specs are very explicit and verbose. Is that intentional and
necessary? (And %{_contribdir} is listed twice.)
second _contribdir removed
it seems to me there is quite a lot of space for improvement since attributes
do not need to be specified twice (install in %install and %files), I'll look
at this.
Rpmlint says
6 packages and 1 specfiles checked; 20 errors, 61 warnings.
Some of the warnings might be invalid, but some of them definitely should be
adressed before review.
I've lowered the number a little for now
The spec has 30 sources and 15 patches without any indication if they have
been
pushed upstream.
Afaik they were rejected
some sources are additional modules/tools that upstream is not interested in
--
This is first round and definitely not finished. Just to show you there is
someone on the other end. I'll continue with this on monday
---
changes were only commited, not tagged yet, you find actual (not finished) spec
in cvs
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the QA contact for the bug.
___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 231861] Review Request: cyrus-imapd - high-performance mail server (IMAP, POP3, ...)
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=231861 --- Comment #12 from Mads Kiilerich m...@kiilerich.com 2009-10-29 12:30:49 EDT --- The spec has 30 sources and 15 patches without any indication if they have been pushed upstream. Afaik they were rejected Probably. I worked a bit on packaging 10 (hmm ... scary!) years ago (http://www.rpmfind.net/linux/RPM/conectiva/atualizacoes/8/RPMS/cyrus-imapd-devel-static-2.0.17-1U80_1cl.i386.html), and back then upstream wasn't that open. But the license change might be an indication that things have changed now? some sources are additional modules/tools that upstream is not interested in That might be. But there are so many of them that it almost deserves a real home. Fedora CVS is not a good upstream. Perhaps upstream could be convinced to carry it in its contrib folder? -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 231861] Review Request: cyrus-imapd - high-performance mail server (IMAP, POP3, ...)
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=231861 --- Comment #13 from Jason Tibbitts ti...@math.uh.edu 2009-10-29 13:00:54 EDT --- I'm glad to see some progress with cyrus-imapd, or any merge review for that matter, but is anyone actually reviewing this? fedora-review is set to '?' but the ticket isn't assigned to anyone. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 231861] Review Request: cyrus-imapd - high-performance mail server (IMAP, POP3, ...)
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=231861
Mads Kiilerich m...@kiilerich.com changed:
What|Removed |Added
CC||m...@kiilerich.com
--- Comment #10 from Mads Kiilerich m...@kiilerich.com 2009-10-27 14:11:02
EDT ---
Is this package still ready for review? (And shouldn't it be a Merge Review?)
How can it be that it has status Assigned but isn't assigned to anybody?
My first impression is that it looks like it haven't been dressed up for examn
and could use some polishing before a final review.
Some brief comments:
It seems like most (all?) of the code now is licensed announcement-BSD-ish, so
the License and the comments about it are a bit misleading.
The spec is quite complex and verbose and IMHO not easy to read.
The spec contains comments left over from the Invoca version.
_perlhack variable seems to be unused since 7.3 - Red Hat, not Fedora!
There are manu variables and configuration options. Are they necessary and
used?
The %file specs are very explicit and verbose. Is that intentional and
necessary? (And %{_contribdir} is listed twice.)
Rpmlint says
6 packages and 1 specfiles checked; 20 errors, 61 warnings.
Some of the warnings might be invalid, but some of them definitely should be
adressed before review.
The spec has 30 sources and 15 patches without any indication if they have been
pushed upstream.
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the QA contact for the bug.
___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 231861] Review Request: cyrus-imapd - high-performance mail server (IMAP, POP3, ...)
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=231861 Jay Turner [EMAIL PROTECTED] changed: What|Removed |Added AssignedTo|[EMAIL PROTECTED] |[EMAIL PROTECTED] -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 231861] Review Request: cyrus-imapd - high-performance mail server (IMAP, POP3, ...)
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/show_bug.cgi?id=231861 [EMAIL PROTECTED] changed: What|Removed |Added Flag||fedora-cvs? --- Additional Comments From [EMAIL PROTECTED] 2007-10-15 11:02 EST --- Package Change Request == Package Name: cyrus-imapd New Branches: F-8 -- Configure bugmail: https://bugzilla.redhat.com/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@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 231861] Review Request: cyrus-imapd - high-performance mail server (IMAP, POP3, ...)
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/show_bug.cgi?id=231861 [EMAIL PROTECTED] changed: What|Removed |Added Flag|fedora-cvs? |fedora-cvs+ --- Additional Comments From [EMAIL PROTECTED] 2007-10-15 11:29 EST --- cvs done. -- Configure bugmail: https://bugzilla.redhat.com/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@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 231861] Review Request: cyrus-imapd - high-performance mail server (IMAP, POP3, ...)
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/show_bug.cgi?id=231861 [EMAIL PROTECTED] changed: What|Removed |Added Flag|fedora-cvs? | -- Configure bugmail: https://bugzilla.redhat.com/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@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 231861] Review Request: cyrus-imapd - high-performance mail server (IMAP, POP3, ...)
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/show_bug.cgi?id=231861 [EMAIL PROTECTED] changed: What|Removed |Added Flag|fedora-cvs+ |fedora-cvs? --- Additional Comments From [EMAIL PROTECTED] 2007-10-15 12:45 EST --- Lubo, Thanks for your comments and sorry for the delay. Me not like reviews, you know. I did a few changes as you suggested: http://cvs.fedora.redhat.com/viewcvs/devel/cyrus-imapd/cyrus-imapd.spec?root=extrasr1=1.32r2=1.33 And I have a few comments: Ad 2). I'd like to leave the HTML docs in the main package, but you may try to convince me. I don't think we should delete the HTML manpages though, because they are linked from the HTML docs. Ad 3). As you pointed out, there are no shared libs in the package. libcyrus is intended to be linked statically and that's what is in the -devel subpackage. Ad 4). I fixed these and about 1 other, will maybe fix others later. There has been a change in the License field since, re-review this, please. And, the most important thing: This package has a whole lot of files in /usr/lib/cyrus-imapd, regardless of the architecture. This used to be a reason for marking it multiarch and warning about multiarch collisions. It's been added to a list of exceptions since then. But the question remains -- should I leave it that way, or should I make some effort to move those to /usr/libexec (or something similar)? -- Configure bugmail: https://bugzilla.redhat.com/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@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 231861] Review Request: cyrus-imapd - high-performance mail server (IMAP, POP3, ...)
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/show_bug.cgi?id=231861 --- Additional Comments From [EMAIL PROTECTED] 2007-10-03 17:03 EST --- Ping on this. -- Configure bugmail: https://bugzilla.redhat.com/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@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 231861] Review Request: cyrus-imapd - high-performance mail server (IMAP, POP3, ...)
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
[EMAIL PROTECTED] changed:
What|Removed |Added
Severity|normal |medium
Priority|normal |medium
Product|Fedora Extras |Fedora
[EMAIL PROTECTED] changed:
What|Removed |Added
AssignedTo|[EMAIL PROTECTED]|[EMAIL PROTECTED]
Flag||fedora-review?
--- Additional Comments From [EMAIL PROTECTED] 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@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 231861] Review Request: cyrus-imapd - high-performance mail server (IMAP, POP3, ...)
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 [EMAIL PROTECTED] changed: What|Removed |Added Status|NEW |ASSIGNED -- 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@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 231861] Review Request: cyrus-imapd - high-performance mail server (IMAP, POP3, ...)
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 --- Additional Comments From [EMAIL PROTECTED] 2007-06-28 14:49 EST --- Created an attachment (id=158151) -- (https://bugzilla.redhat.com/bugzilla/attachment.cgi?id=158151action=view) Rpmlint results for cyrus-imapd x86_64, source and subpackages -- 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@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
