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 #17 from Michal Hlavinka <mhlavink@xxxxxxxxxx> 2009-11-26 08:39:45 EDT --- (In reply to comment #16) > Ok, another round of comments/questions: > > MIT license? AFAICS the COPYRIGHT file is BSD. The CMU entry on > http://fedoraproject.org/wiki/Licensing is internally inconsistent, and > http://fedoraproject.org/wiki/Licensing/MIT#CMU_Style isn't the cyrus license - > AFAIK that license has never applied to cyrus. oops, you're right, it's BSD license fixed > _cyrusconf seems like a bit of overkill. this was here to set config file version (prefork/no-prefork), but we did not change that for ages, removed fixed > > Haven't all the versions in the requirements been irrelevant for ages? > verified and removed fixed > Shouldn't most of the renamings and other file hackings in %build be in %prep? > fixed > Why this "if pkg-config openssl" thing? When and why does it apply? > (afaik) this was there because there were problems with makefiles, but it's not needed now, removed fixed > pushd before "perl -pi" and use of "$(ls *x)" isn't needed - I would prefer the > simple version > fixed > It seems like there is no need for cleanup of "*~" and "*.html.*". > fixed > Why do %pre stop the service but pretend it is running? > I guess it's for keeping condrestart working. I don't know why there is not just condrestart itself, but I thought it's because there were some race conditions. After searching through cvs history and filled bugs, I was not able to find any complain that condrestart alone does not work. removed fixed > Is the chattr in %post OK? It might be a good idea, but is it OK that a package > does it automatically? Shouldn't it be in README.RPM instead? Anyway, why not > just > chattr -R +S %{_var}/lib/imap/{user,quota} %{_var}/spool/imap > afaik cyrus-imapd expect this behaviour and I don't think this makes any problems. Simplified fixed > Shouldn't the certificate creation be moved to service start where it is more > transparent what goes on, just like /etc/rc.d/init.d/sshd does? > well, would it make any real difference? For example dovecot does the same think. Yes, I know it's bad example since I'm its owner, but I didn't add that part in specs and don't know any other package that creates certificates after installation (except dovecot, cyrus-imapd and openssh). > Why is the user and group (and csync) created by the utils package %pre which > doesn't use them? Move to main package? > well, just a little complicated here. cyrus-imapd requires utils and some of them are executed as cyrus user. But yes, user creation should be in main package. Moved fixed > Wouldn't it be better if csync were included in the setup packages > /etc/services? > I've asked setup maintainer to add csync in /etc/services ( http://git.fedorahosted.org/git/?p=setup.git;a=summary ) waiting for new setup release > Is it relevant to include html versions of the man pages? they are making no harm, but yes, they are not necessary, removed. fixed > > Much of README.RPM is outdated or irrelevant. I think "chkconfig cyrus-imapd > on" should be mentioned. README.buildoptions doesn't exist. And postfix > shouldn't be discriminated ;-) this file seems outdated and not too useful for Fedora, removed fixed -------- btw, there is "discussion" on mailing list where someone asked for including autocreate patch in upstream's repository. I've added there my "me too". Watching for upstream's decision. -- 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@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review