Bug 453839

In case you didn't know, you should be able to avoid all of that nasty %lang
business by using the %find_lang macro.  See
http://fedoraproject.org/wiki/Packaging/Guidelines#Why_do_we_need_to_use_.25find_lang.3F

Update :
Spec URL: http://nicoleau.fabien.free.fr/rpms/SPECS/phatch.spec
SRPM URL:
http://nicoleau.fabien.free.fr/rpms/srpms.fc9/phatch-0.1.4.bzr538-2.fc9.src.rpm

I'm now using %find_lang macro.

Rebuild under mock is OK.

rpmlint output :
[builder@FEDOBOX tmp]$ rpmlint phatch-0.1.4.bzr538-2.fc9.noarch.rpm 
phatch.noarch: E: only-non-binary-in-usr-lib
1 packages and 0 specfiles checked; 1 errors, 0 warnings.
[builder@FEDOBOX tmp]$ rpmlint phatch-0.1.4.bzr538-2.fc9.src.rpm 
phatch.src:73: W: libdir-macro-in-noarch-package
%{_libdir}/nautilus/extensions-1.0/python/%{name}_*
1 packages and 0 specfiles checked; 0 errors, 1 warnings.
[builder@FEDOBOX tmp]$ 

Update for 0.1.5 :
Spec URL: http://nicoleau.fabien.free.fr/rpms/SPECS/phatch.spec
SRPM URL:
http://nicoleau.fabien.free.fr/rpms/srpms.fc9/phatch-0.1.5-1.fc9.src.rpm


Rebuild under mock is OK.
rpmlint output :
[builder@FEDOBOX tmp]$ rpmlint phatch-0.1.5-1.fc9.noarch.rpm
phatch-0.1.5-1.fc9.src.rpm
phatch.noarch: E: only-non-binary-in-usr-lib
phatch.src:76: W: libdir-macro-in-noarch-package
%{_libdir}/nautilus/extensions-1.0/python/%{name}_*
2 packages and 0 specfiles checked; 1 errors, 1 warnings.
[builder@FEDOBOX tmp]$ 

Just took a glimpse at the program's site and it seems that the license is
GPLv3+. Most of the .py files I've seen either have no license or contain the
following:
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation, either version 3 of the License, or
# (at your option) any later version

Update (licence is now GPLv3+) :
Spec URL: http://nicoleau.fabien.free.fr/rpms/SPECS/phatch.spec
SRPM URL:
http://nicoleau.fabien.free.fr/rpms/srpms.fc9/phatch-0.1.5-2.fc9.src.rpm

Rebuild under mock is OK.
rpmlint output :
[builder@FEDOBOX tmp]$ rpmlint phatch-0.1.5-2.fc9.noarch.rpm
phatch-0.1.5-2.fc9.src.rpm
phatch.noarch: E: only-non-binary-in-usr-lib
phatch.src:76: W: libdir-macro-in-noarch-package
%{_libdir}/nautilus/extensions-1.0/python/%{name}_*
2 packages and 0 specfiles checked; 1 errors, 1 warnings.
[builder@FEDOBOX tmp]$ 

This rpmlint complaint:
  phatch.src:76: W: libdir-macro-in-noarch-package 
   %{_libdir}/nautilus/extensions-1.0/python/%{name}_*
is an absolute blocker.

If you build this noarch package on x86_64, you'll get files in /usr/lib64, which doesn't even exist on a 32-bit machine.  I do not know what the proper solution is; if nautilus really has no place to put arch-independent extensions then I suppose this package can't be noarch.

Update for 0.1.6 :
Spec URL: http://nicoleau.fabien.free.fr/rpms/SPECS/phatch.spec
SRPM URL:
http://nicoleau.fabien.free.fr/rpms/srpms.fc9/phatch-0.1.6-1.fc9.src.rpm

Its no more a noarch package, has %{_libdir} is used.
No debuginfo generated.

Package build on koji : http://koji.fedoraproject.org/koji/taskinfo?taskID=923733
rpmlint output :

[eponyme@FEDOBOX tmp]$ rpmlint phatch-0.1.6-1.fc9.i386.rpm phatch-0.1.6-1.fc9.src.rpm 
phatch.i386: E: no-binary
phatch.i386: E: only-non-binary-in-usr-lib
2 packages and 0 specfiles checked; 2 errors, 0 warnings.
[eponyme@FEDOBOX tmp]$

Two comments (I'll do a full review today or tomorrow):

* The nautilus extension dependency pulls almost the entire gnome suite. The KDE (or other DE) user don't need that. I think it should go into a subpackage.

* Please preserve the timestamp of the sed'ed file.

OK, here is the full review. In addition to the above two things:

- rpmlint errors can be ignored.

* I don't think you need to BR: wxPython-devel . The package builds fine without it.

* Source0 link is broken. Does this change frequently? If yes, I guess you should just put the filename, not the full URL (but in this case, make a comment in the SPEC file).

* Any reason why you skipped packaging docs/phatch_dev.txt in %doc ?

* Please follow the desktop-database and mimeinfo sections (4.6 and 4.7) at
    http://fedoraproject.org/wiki/Packaging/ScriptletSnippets

* The python egg shouldn't be excluded, but it should be built in a proper way. This is a non-setuptools package. So follow:
    http://fedoraproject.org/wiki/Packaging/Python/Eggs#Providing_Eggs_for_non-setuptools_packages

Thank you for taking this review.
Update :
Spec URL: http://nicoleau.fabien.free.fr/rpms/SPECS/phatch.spec
SRPM URL:
http://nicoleau.fabien.free.fr/rpms/srpms.fc10/phatch-0.1.6-2.fc10.src.rpm

Changelog :
- subpckage created for nautilus extension
- timestamp preserved for sed'ed file
- wxPython-devel removed from BuildRequires
- Source0 updated
- whole documentation included
- update-desktop-database and update-mime-database added
- egg-info properly builded
- python-devel buildrequries changed to python (whithout that, build uner mock/koji failed)

Source0 is a "stable" URL. The url changed from far but I didn't have update it in my SPEC.
I tried to use scrollkeeper but no additionnal documentation was generated, so I removed it. 
I also contacted upstream about the software version (egg file generated contains "0.1.5" instead of "0.1.6" and the "about" dialog display also "0.1.5" instead of "0.1.6").

Package build on koji :  https://bugzilla.redhat.com/show_bug.cgi?id=453839
repmlint output :
[builder@FEDOBOX ~]$ rpmlint /home/builder/rpmbuild/SRPMS/phatch-0.1.6-2.fc10.src.rpm /home/builder/rpmbuild/RPMS/i386/phatch-0.1.6-2.fc10.i386.rpm /home/builder/rpmbuild/RPMS/i386/phatch-nautilus-extension-0.1.6-2.fc10.i386.rpm
phatch.i386: E: no-binary
phatch-nautilus-extension.i386: W: no-documentation
phatch-nautilus-extension.i386: E: only-non-binary-in-usr-lib
3 packages and 0 specfiles checked; 2 errors, 1 warnings.
[builder@FEDOBOX ~]$

oops, here is the good URL for koji build :
http://koji.fedoraproject.org/koji/taskinfo?taskID=1009427

(In reply to comment #10)
> Thank you for taking this review.

You're welcome

> Changelog :
> - subpckage created for nautilus extension

I am a gnome-ignorant. How do I test this extension? I couldn't find anything in nautilus related to phatch. But it's probably me. I always find gnome too confusing.

   Requires: %{name} = %{version}
should be 
   Requires: %{name} = %{version}-%{release}

Also, please add the license file at the least as a %doc to this package. 

> - whole documentation included
Thanks. But now there is a docs subdirectory in a directory which is already for docs. I don't think this is elegant. Try using "docs/*" in %doc instead of just "docs"

> I tried to use scrollkeeper but no additionnal documentation was generated, so
> I removed it. 

Sorry I meant sections 4.7 or 4.8 (or they just added another section between my review and you seeing it)

> I also contacted upstream about the software version (egg file generated
> contains "0.1.5" instead of "0.1.6" and the "about" dialog display also "0.1.5"
> instead of "0.1.6").
> 

Until they fix it (possibly until next version), you can use:
   sed -e 's|0\.1\.5|0\.1\.6|' -e 's|20080606224435|200811091037|' \
       phatch/data/version.py > phatch/data/version.py.16
   touch -c -r phatch/data/version.py phatch/data/version.py.16
   mv phatch/data/version.py.16 phatch/data/version.py
This will fix both the python egg and the about dialog.

Update :
Thank you for taking this review.
Update :
Spec URL: http://nicoleau.fabien.free.fr/rpms/SPECS/phatch.spec
SRPM URL:
http://nicoleau.fabien.free.fr/rpms/srpms.fc10/phatch-0.1.6-3.fc10.src.rpm

Changelog :
- Fix requires for nautilus-extension
- Licence file added to nautilus-extension
- No more docs subdirectory in documentation folder
- Fix version

> I am a gnome-ignorant. How do I test this extension? I couldn't find anything
> in nautilus related to phatch. But it's probably me. I always find gnome too
> confusing.

To test the extension : right-clicking must display 2 entries related to phatch in the context-menu. Uninstalling phatch-nautilus-extension will make those entries disapears (You have to log{on/off} when you (un)install the extension to see differencies).

Package still build on koji : http://koji.fedoraproject.org/koji/taskinfo?taskID=1010031

Rpmlint output :
[builder@FEDOBOX rpmbuild]$ rpmlint /home/builder/rpmbuild/SRPMS/phatch-0.1.6-3.fc10.src.rpm /home/builder/rpmbuild/RPMS/i386/phatch-0.1.6-3.fc10.i386.rpm /home/builder/rpmbuild/RPMS/i386/phatch-nautilus-extension-0.1.6-3.fc10.i386.rpm
phatch.i386: E: no-binary
phatch-nautilus-extension.i386: E: only-non-binary-in-usr-lib
3 packages and 0 specfiles checked; 2 errors, 0 warnings.
[builder@FEDOBOX rpmbuild]$

Everything is good, but running a quick search I found:
 
 $ yum search nautilus |grep extension
 nautilus-actions.x86_64 : Nautilus extension for customizing the context menu
 nautilus-extensions.i386 : Nautilus extensions library
 nautilus-extensions.x86_64 : Nautilus extensions library
 nautilus-image-converter.x86_64 : Nautilus extension to mass resize images
 nautilus-open-terminal.x86_64 : Nautilus extension for an open terminal shortcut
 nautilus-search-tool.x86_64 : A Nautilus extension to put "Search for Files" on
 nautilus-sound-converter.x86_64 : Nautilus extension to convert audio files

So, I guess renaming the subpackage as nautilus-%{name} would be more appropriate. What do you think?

You can do this before you commit.

-----------------------------------------
This package (phatch) is APPROVED by oget
-----------------------------------------

Thank you very much for this review.
I'll change subpackage name to nautilus-%{name} before I commit.

New Package CVS Request
=======================
Package Name: phatch
Short Description: Photo batch processor
Owners: eponyme
Branches: F-9 F-10
InitialCC:

PS : after a test, nautilus-%{name} will, in fact, generate phatch-nautilus-phatch, so I think I must only keep "nautilus" to have "phatch-nautilus".

For that, you'll need to use the "-n" switch
For instance
%package -n nautilus-%{name}
%description -n nautilus-%{name}
%files -n nautilus-%{name}


(Doesn't apply to this package but you can use "-n" switch on %pre(un) %post(un) too)

cvs done.

As I'm on hoolydays, I'll import phatch next week :D

phatch-0.1.6-3.fc9 has been submitted as an update for Fedora 9.
http://admin.fedoraproject.org/updates/phatch-0.1.6-3.fc9

phatch-0.1.6-3.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/phatch-0.1.6-3.fc10

phatch-0.1.6-3.fc10 has been pushed to the Fedora 10 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 phatch'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2008-11859

phatch-0.1.6-3.fc9 has been pushed to the Fedora 9 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-newkey update phatch'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F9/FEDORA-2008-11911

phatch-0.1.6-3.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.

phatch-0.1.6-3.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.