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: strigi - A desktop search program for KDE https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=223586 ------- Additional Comments From Kevin@xxxxxxxxxxxxxxxx 2007-05-04 19:40 EST ------- MUST Items: + rpmlint output OK (see comment #22) + named and versioned according to the Package Naming Guidelines + spec file name matches base package name + Packaging Guidelines: + License LGPL OK + No known patent problems + No emulator, no firmware, no binary-only or prebuilt components + Complies with the FHS + proper changelog, tags, BuildRoot, Requires, BuildRequires, Summary, Description + no non-UTF-8 characters + relevant documentation is included + RPM_OPT_FLAGS are used + debuginfo package is valid + no static libraries nor .la files + no duplicated system libraries + no rpaths, at least on i386 (I ran strings on a few files) + no configuration files, so %config guideline doesn't apply + no init scripts, so init script guideline doesn't apply + desktop file is valid and installed correctly + no timestamp-clobbering file commands + _smp_mflags used + scriptlets are valid + not a web application, so web application guideline doesn't apply + complies with all the legal guidelines MUST FIX: The License tag says "GPL", but the license is actually LGPL according to COPYING. + COPYING included as %doc + spec file written in American English + spec file is legible + source matches upstream: MD5SUM: b976b4f3cf451fc53cd773c338d78994 SHA1SUM: 0bc32ab1668492ad44d061fd4d5753ec0344cb41 + builds on at least one arch (FC6 i386) + no known non-working arches, so no ExcludeArch needed + all build dependencies listed in CMakeLists.txt are listed in BuildRequires, however: SHOULD FIX: openssl-devel is no longer required, please remove BR SHOULD FIX: expat-devel is not actually required either: CMakeLists.txt requires libxml2-devel, then goes on to require expat-devel OR libxml2-devel, so this actually means libxml2-devel is required and expat-devel is not really used (and rpm -q --requires strigi confirms that) + no translations in original tarball, so translation/locale guidelines don't apply + ldconfig correctly called in %post and %postun + package not relocatable + ownership correct (owns package-specific directories, doesn't own directories owned by another package) except for the following: MUST FIX: strigiclient.desktop is installed into /usr/share/applications/kde. This is owned by kdelibs, which is not a direct or indirect dependency of strigi. So this has to be fixed one way or the other. As Strigi is in no way a KDE app (strigiclient is a Qt 4 app), please simply install the .desktop file into /usr/share/applications instead. + no duplicate files in %files + permissions set properly + %clean section present and correct + macros used where possible (%cmake macro does not work according to comment #18) + no non-code content + no large documentation files + %doc files not required at runtime + all header files in -devel + no static libraries, so no -static package needed + Requires: pkgconfig present + /usr/lib/*.so symlinks are correctly in -devel, however: MUST FIX: The .so files in /usr/lib/strigi are NOT symlinks, they're loadable plugins, so they must be in the main package, not the -devel package. + -devel requires %{name} = %{version}-%{release} + no .la files + the only GUI program in the package, strigiclient, has a .desktop file + buildroot is deleted at the beginning of %install + all filenames are valid UTF-8 SHOULD Items: + license already included upstream + no translations for description and summary provided by upstream + package builds in mock (or at least used to) according to comment #1 (I may run the current package through mock later, but that's best done once the redundant BRs are removed so we can be sure they really were redundant. ;-) ) + package builds successfully on my live system, and I checked for missing BRs (found none) + can't test non-i386 architectures, but all arches are supposed to be supported by upstream + package appears to work + scriptlets are sane + no subpackages other than -devel, so "Usually, subpackages other than devel should require the base package using a fully versioned dependency." is irrelevant + .pc files are in -devel + no file dependencies Nitpicks: * The guidelines say the Summary should not end with a period. This, however, doesn't apply to the %description. The sentence at the end of your main package %description should end with a period. (The %description devel is not a complete sentence and thus should not end with a period though.) * Why did you set strigiclient.desktop to only get shown in KDE? * "accessesed" should read "accessed". * This one's a question more than a request to fix something: How is strigidaemon supposed to get run? Is this supposed to be done by the clients which need it (such as strigiapplet)? The strigiclient in any case doesn't offer to start the daemon, it just spits out warnings on the console when run without the daemon running, and some operations even crash in that case. Not that I personally care, I only need the indexers to build KDE 4 alpha 1 (used by the KDE 4 metadata classes), I hate background-indexing daemons, but people who actually want to use the daemon will need it started somehow. Please fix your License tag, install the .desktop file into just %{_datadir}/applications, move %{_libdir}/strigi/ from -devel to the main package and remove the redundant BRs, then I'll approve this. (Also please consider fixing the nitpicks, but those aren't blockers.) -- 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