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=224245 Jason Tibbitts <tibbs@xxxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mhlavink@xxxxxxxxxx, | |tibbs@xxxxxxxxxxx --- Comment #14 from Jason Tibbitts <tibbs@xxxxxxxxxxx> 2008-12-18 22:56:43 EDT --- Looks like mhlavink@xxxxxxxxxx owns this package now, so I'll update the CC. I've been staring at this, the very first merge review, for nearly two years now and would love to see it go away. I'm not sure if I have what it takes to do a proper review of it, but I can at least look at the current packaging and make some comments. First off, I wonder how much of the above discussion about Fedora's version being heavily patched is currently true. I see only four patches applied; one is a one-liner, one is a set of translation changes for, I believe, Turkish, and the other two don't really patch in all that much (under fifteen lines each). It would indeed be nice to include the upstream status of those patches in accordance with http://fedoraproject.org/wiki/Packaging/Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment but I'm just not seeing how our current version is significantly patched. Maybe someone did some significant work on things in the last 18 months. Or maybe the problem is with some of the locale tweaks that happen directly in the specfile. It would also be nice to clean the patches up a bit, remove the unapplied patches from CVS and get rid of the commented-out bits of the spec where those patches used to be applied. Here's what current rpmlint has to say about the src.rpm: 37: E: prereq-use httpd, perl Prereq: shouldn't be used; something is needed for a scriptlet, it should be reflected in a fine-grained dependency like Requires(pre). There are no scriptlets here, so I think those should be regular runtime dependencies, specified with Requires:. Those happen to already be there, so I don't really see the point in the Prereq: line. 38: W: unversioned-explicit-provides squirrelmail-i18n We always want to provide something with a version, because without one it becomes impossible to ever provide a separate squirrelmail-i18n package. However, is there anything which might actually depend on squirrelmail-i18n? Why does it need to be provided by this package? 217: E: use-of-RPM_SOURCE_DIR Shouldn't this just refer to %{SOURCE1} instead? W: mixed-use-of-spaces-and-tabs (spaces: line 76, tab: line 76) Nobody cares about this. Fix it if you like. and about the built noarch.rpm: W: non-standard-uid /var/lib/squirrelmail/prefs apache W: non-standard-gid /var/lib/squirrelmail/prefs apache E: non-standard-dir-perm /var/lib/squirrelmail/prefs 0700 W: non-standard-uid /var/spool/squirrelmail/attach apache W: non-standard-gid /var/spool/squirrelmail/attach apache E: non-standard-dir-perm /var/spool/squirrelmail/attach 0700 W: non-standard-gid /etc/squirrelmail/config.php apache E: non-readable /etc/squirrelmail/config.php 0640 W: non-standard-gid /etc/squirrelmail/config_local.php apache E: non-readable /etc/squirrelmail/config_local.php 0640 W: non-standard-gid /etc/squirrelmail/default_pref apache E: non-readable /etc/squirrelmail/default_pref 0640 W: non-standard-gid /etc/squirrelmail/sqspell_config.php apache E: non-readable /etc/squirrelmail/sqspell_config.php 0640 These are all perfectly OK. E: htaccess-file /usr/share/squirrelmail/plugins/squirrelspell/modules/.htaccess Generally it's a bad idea to use a .htaccess file in a package; better to put such things in /etc/httpd/conf.d/squirrelmail.conf so the admin can see all of the access restrictions in one place. It's also common to disable htaccess checks for reasons of speed. W: file-not-utf8 /usr/share/doc/squirrelmail-1.4.17/ChangeLog It would be significantly better if anyone who gets this package would be able to actually read the names in the ChangeLog, but such things tend to get messy and bits in various different character sets are often added at different times. Fixing it is generally a manual procedure and needs to be coordinated with upstream (because generally iconv can't fix these problems and carrying a patch to the ChangeLog is pointless). I don't think the package review is the appropriate place to force that to happen. W: wrong-file-end-of-line-encoding /usr/share/doc/squirrelmail-1.4.17/ReleaseNotes/1.4/Notes-1.4.12.txt W: wrong-file-end-of-line-encoding /usr/share/doc/squirrelmail-1.4.17/ReleaseNotes/1.4/Notes-1.4.13.txt These, however, should be fixed with a couple of quick calls to tr. E: zero-length /usr/share/squirrelmail/locale/es_ES/LC_MESSAGES/serversidefilter.po.new Why do we ship this? W: devel-file-in-non-devel-package /usr/share/squirrelmail/plugins/filters/bulkquery/bulkquery.c E: non-executable-script /usr/share/squirrelmail/plugins/demo/getpot 0644 What are these for? If they're not usable as is, perhaps they should be removed from the actual squirrelmail installation and packaged as documentation. That's it for rpmlint. I'll look at the spec later if anyone is listening on the other end of this ticket. -- 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