Nice a python3 package :-)

The loook-0.6.7-python3-env.patch is wrong, you force to use python3.1, but what's up, when python3.2 arrives?
Better just use python3 there and that's it...

- %files: sitelib and not sitearch

- compressing of the manpage happens automatically, so please just install %{name}.1 and don't use gzip

- you do:
  %py_byte_compile %{__python3} %{buildroot}%{_datadir}/%{name}/

  But there are no files there... Can this be deleted?

- %global         debug_package %{nil}:
  that's not needed for noarch packages, please delete that.

- license is wrong: it's GPLv2+ and not just GPLv2

The rest looks fine to me...

Hi Lukáš,

As discussed I'll review this and 2 more of you're packages and then sponsor you.

Thomas, sorry for taking over this review from you and thanks for the initial review. The reason I'm hijacking this review is that Lukáš needs a sponsor (but forgot to set the FE-NEEDSPONSOR blocker on this bug) and I've agreed to sponsor him, see bug 635450.

Thanks & Regards,

Hans

Good:
- rpmlint checks return:
  2 packages and 0 specfiles checked; 0 errors, 0 warnings.
- package meets naming guidelines
- package meets packaging guidelines
- spec file legible, in am. english
- source matches upstream
- package compiles on devel (x86)
- no missing BR
- no unnecessary BR
- no locales
- not relocatable
- owns all directories that it creates
- no duplicate files
- permissions ok
- %clean ok
- macro use consistent
- code, not content
- no need for -docs
- nothing in %doc affects runtime

Needs work:
- All comments from Thomas' comment #1 need to be addressed, wrt to the
  bytecompiling I would like to add that doing explicit byte compiling from
  the .spec is never needed as rpmbuild automatically bytecompiles .py files.
- This app needs a .desktop file. As already discussed by email, I think
you can simply use the standard search icon from the active icon theme
for it (usually a spyglass in some form). Simply put the following in the
.desktop file:
Icon=edit-find

You can find all the standardized icon theme icon names here:
http://standards.freedesktop.org/icon-naming-spec/icon-naming-spec-latest.html

Usually that is not really a good solution for an apps .desktop
file, but since this is a search tool ...

[lzap@lzapx SRPMS]$ rpmlint loook-0.6.7-3.fc14.src.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

[lzap@lzapx noarch]$ rpmlint loook-0.6.7-3.fc14.noarch.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

All solved.

http://static.zapletalovi.com/fedora/rpm/loook/0.6.7-3/

Many thanks for your help!

Hi,

Sorry to be a bit pedantic, but I would like to see one more revision.

This bit:

install -Dpm 0644 %{name}.desktop $RPM_BUILD_ROOT%{_datadir}/applications/%{name}.desktop

desktop-file-install --delete-original  \
        --dir $RPM_BUILD_ROOT%{_datadir}/applications   \
        --remove-category Application \
        $RPM_BUILD_ROOT%{_datadir}/applications/%{name}.desktop

Is a bit weird, first you install it using install, then you install it *again* using desktop-file-install. Instead you can simply do:

desktop-file-install --dir $RPM_BUILD_ROOT%{_datadir}/applications   \
        %{name}.desktop

Note:
1) The --delete-original is gone now, as it is no longer needed
2) The --remove-category Application is gone (it is only needed
   for older .desktop files which wrongly contain Application in
   their Categories=...; list)
3) It may be necessary to first create
   $RPM_BUILD_ROOT%{_datadir}/applications
4) Adding a .desktop file with a patch is a bit unconventional. It is fine
   but usually people just include the file as

Source1: %{name}.desktop

   And the install command becomes:

desktop-file-install --dir $RPM_BUILD_ROOT%{_datadir}/applications %{SOURCE1}

Regards,

Hans

No problem. Still learning... Sorry.

http://static.zapletalovi.com/fedora/rpm/loook/0.6.7-4/

(In reply to comment #6)
> No problem. Still learning...

I understand, no problem from my side either, and no need to say sorry :)

All is good now: Approved!

New Package SCM Request
=======================
Package Name: loook
Short Description: Search tool for OpenOffice.org documents
Owners: lzap
Branches: f14
InitialCC:

Git done (by process-git-requests).

loook-0.6.7-5.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/loook-0.6.7-5.fc14

loook-0.6.7-5.fc14 has been pushed to the Fedora 14 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update loook'.  You can provide feedback for this update here: https://admin.fedoraproject.org/updates/loook-0.6.7-5.fc14

loook-0.6.7-5.fc14 has been pushed to the Fedora 14 stable repository.  If problems still persist, please make note of it in this bug report.