Bug 688659

I'll go ahead and review this when I get a chance here.

Setting flag and starting in on a review. Look for a review later today. :)

OK - Package meets naming and packaging guidelines
OK - Spec file matches base package name. 
OK - Spec has consistant macro usage. 
OK - Meets Packaging Guidelines. 
See below - License
See below- License field in spec matches
See below- License file included in package
OK - Spec in American English
OK - Spec is legible.
OK - Sources match upstream md5sum:
6154df9ab701ec3aba6de251011a3d00  thunarx-python-0.2.3.tar.bz2
6154df9ab701ec3aba6de251011a3d00  thunarx-python-0.2.3.tar.bz2.rpm

See below - BuildRequires correct
OK - Package has %defattr and permissions on files is good. 
OK - Package has a correct %clean section. 
OK - Package has correct buildroot
OK - Package is code or permissible content. 
OK - Packages %doc files don't affect runtime. 
OK - Package has rm -rf RPM_BUILD_ROOT at top of %install

See below - .la files are removed. 

OK - Package compiles and builds on at least one arch. 
OK - Package has no duplicate files in %files. 
OK - Package doesn't own any directories other packages own. 
OK - Package owns all the directories it creates. 
OK - Package obey's FHS standard (except for 2 exceptions)
See below - No rpmlint output. 
See below - final provides and requires are sane.

SHOULD Items:

OK - Should build in mock. 
OK - Should build on all supported archs
OK - Should function as described. 
OK - Should have sane scriptlets. 
OK - Should have subpackages require base package with fully versioned depend. 
OK - Should have dist tag
OK - Should package latest version
OK - Should not use file requires outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin

Issues: 

1. The license doesn't seem right in the spec file. 
Look at the head of the source files in the src/ 
directory. 

2. You have no %doc files. Would be good to add AUTHORS ChangeLog docs/ COPYING NEWS README

3. There shouldn't be any need for the 
Requires: Thunar pygtk2 pygobject2 gnome-python2
line. When something is linked against things, rpm is smart enough to add the requires
in for you. ;) 

4. You might want to build the gtk-doc's here. 
checking whether to build gtk-doc documentation... no
checking for gtkdoc-check... no
(Should be BuildRequires: gtk-doc and adding produced files)

5. The .la files are still there. You need to change
rm -f $RPM_BUILD_ROOT%{_libdir}/*.la
to
rm -f $RPM_BUILD_ROOT%{_libdir}/*/*.la
And make sure it's after the install call. 

6. rpmlint says: 
thunarx-python.x86_64: W: incoherent-version-in-changelog 0.1-1 ['0.2.3-1.fc16', '0.2.3-1']
thunarx-python.x86_64: W: no-documentation

The first, your version in the changelog is wrong. You have 0.1-1, but it should be 0.2.3-1
The second will be fixed by adding docs. 

Remember to add a new changelog entry and change version to -2 when you fix the above. 
Thanks!

Kevin,

Thanks a lot for your exhaustive review comments and for your patience to look through my work. I have uploaded the SPEC and the SRPM to the location

http://balajig8.fedorapeople.org/packages/thunarx-python.spec

http://balajig8.fedorapeople.org/packages/thunarx-python-0.2.3-1.fc14.src.rpm

Thanks,
Cheers,
  - Balaji

Thanks for the fixes! 

1. This still isn't quite right... this has the 'or any later version', so it should be 'LGPLv2+' as the License tag. 

2, 3, 4, 5 all look good and fixed. 

6. rpmlint now says: 
thunarx-python.src: W: invalid-license LGPL
thunarx-python.x86_64: W: invalid-license LGPL
thunarx-python-debuginfo.x86_64: W: invalid-license LGPL

which should be fixed by the License change. ;) 

You forgot to change the Release at the top from 1 to 2. ;) 

Fix those two things and I think everything looks good. You can do those two changes before importing the package. This package is APPROVED. I'll go ahead and sponsor you and you can continue the process from: https://fedoraproject.org/wiki/PackageMaintainers/Join#Add_Package_to_Source_Code_Management_.28SCM.29_system_and_Set_Owner once I've added you to the packager group.

Welcome to the fun!

Kevin,

Thanks for the review again and hopefully i believe this should be ok. Have uploaded the spec and SRPMs in the same location as above.

Cheers,
  - Balaji

New Package SCM Request
=======================
Package Name: thunarx-python
Short Description: Python bindings for the Thunar Extension Framework
Owners: balajig8 kevin
Branches: f14 f15 el6
InitialCC: baz

Cheers,
  - Balaji

Git done (by process-git-requests).

Thanks Kevin,Will push the package in.

thunarx-python-0.2.3-3.fc14.1 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/thunarx-python-0.2.3-3.fc14.1

thunarx-python-0.2.3-3.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/thunarx-python-0.2.3-3.fc15

thunarx-python-0.2.3-3.fc15 has been pushed to the Fedora 15 testing repository.

thunarx-python-0.2.3-3.fc15 has been pushed to the Fedora 15 stable repository.

One small issue: The package should not own %{_libdir}/thunarx-2/ but %{_libdir}/thunarx-2/* because the directory is already owned by Thunar.

Not sure if this justifies an update though.

Good catch Christoph. ;) 

Yes, that should be fixed. There is a new upstream release in git, but it's not been pushed out with an announcement or tarball yet. It should be good to fix this when updating to that once it's out.

thunarx-python-0.2.3-3.fc14.1 has been pushed to the Fedora 14 stable repository.