Full review to come, but this is a blocker, according to configure.ac need at
least 2.11.7 of gtk-engines-2 which means that BuildRequires needs to be upped to:
 gtk2-engines >= 2.11.7

This will also mean that it must be build on F-8 and above (F-7 only has
gtk2-engines-2.10.2).

Full review follows: the first section are all MUST fix items:

       - MUST: rpmlint output:
gnome-themes-extras.noarch: W: invalid-license LGPL
gnome-themes-extras.noarch: W: obsolete-not-provided gnome-themes-extras-0.9.0
      - MUST: remove Obsoletes: gnome-themes-themes, don't need to obsolete
packages that have the same name, yum does that by default
      - MUST: don't use %makeinstall, see:
http://fedoraproject.org/wiki/Packaging/Guidelines?highlight=%28%25makeinstall%29
use make install DESTDIR=%{buildroot} instead
      - MUST: License information in the tarball is not clear because README,
COPYING are empty and those that mention specific licenses, e.g Gion and Neu are
GPL not LGPL, please clarify with upstream and read
http://fedoraproject.org/wiki/Licensing if the package contains themes with
different licenses they must each be listed in the license tag, e.g. "License:
GPLv2+ and LGPLv2+" (see also
http://fedoraproject.org/wiki/Packaging/LicensingGuidelines)
      - MUST: As above, License field is currently NOT valid, LGPL is not valid
see: http://fedoraproject.org/wiki/Licensing
      - MUST: COPYING is empty: file bug with upstream asking to include it in
package
      - MUST: BuildRequires: should increase gtk2-engines >= 2.11.7

The following is a checklist of all the OK items:

      - MUST: Meets Package Naming Guidelines.: OK
      - MUST: The spec file name: OK
      - MUST: Meets the Packaging Guidelines: OK
      - MUST: Spec file in American English: OK
      - MUST: The spec file legible: OK
      - MUST: MD5sum matches:
36698ee94c2281d5beab9b0a681f3350  gnome-themes-extras-2.20.tar.gz
http://ftp.gnome.org/pub/GNOME/sources/gnome-themes-extras/2.20/gnome-themes-extras-2.20.md5sum
36698ee94c2281d5beab9b0a681f3350  gnome-themes-extras-2.20.tar.gz
      - MUST: Builds in binary RPM on F-8 i386: OK
      - MUST: uses %find_lang, although not yet needed: OK
      - MUST: ldconfig: not required (noarch): OK
      - MUST: not relocatable: OK
      - MUST: Own all directories that it creates: OK
      - MUST:No duplicate files: OK
      - MUST: %defattr(...): OK
      - MUST: Each package must have a %clean section: OK
      - MUST: Consistently use macros: OK
      - MUST: Contains mainly permissable content, themes: OK
      - MUST: No large doc: OK
      - MUST: %doc not critical for run-time: OK
      - MUST: no devel package: OK
      - MUST: no static libraries: OK
      - MUST: No pkgconfig(.pc) files: OK
      - MUST: no library files with suffix: OK
      - MUST: no .la libtool archives: OK
      - MUST: not a GUI app: OK
      - MUST: Does not own files or directories already owned by other packages: OK
      - MUST: Runs rm -rf %{buildroot}: OK
      - MUST: Filenames valid UTF-8: OK

Hmm, looks like this was orphaned, the old branches are in CVS:

http://cvs.fedoraproject.org/viewcvs/rpms/gnome-themes-extras/

but haven't been marked as dead.package.  Either way, we'll need to re-review
it, so this is good.

The orphaning happened back at FC-6 by the looks of things.  Also: do you have a
sponsor?  I cannot act as a sponsor, although I can act as a reviewer, I believe.

I have fixed all the issues and will upload a new version when I get feedback
from upstream about the licensing and hopefully an explanation relating to the
COPYING file.

No I do not have a sponsor as of yet.  Another package I have submitted is
getting review by Kevin Fenzi.

http://bugzilla.gnome.org/show_bug.cgi?id=496096 as a reference I had to apply
it to a specific theme.

With that in mind and the process of people separating themes out should the
packages be done accordingly as well separated out and licenses applied
individually?

(In reply to comment #7)
> http://bugzilla.gnome.org/show_bug.cgi?id=496096 as a reference I had to apply
> it to a specific theme.
> 
> With that in mind and the process of people separating themes out should the
> packages be done accordingly as well separated out and licenses applied
> individually?

Yes, perhaps they could be split out as subpackages with a separate License: tag
for each one, that might work.  We already have some theme packages like that,
e.g. gnome-theme-clearlooks-bigpack-0.6-6.fc7

(In reply to comment #8)

> Yes, perhaps they could be split out as subpackages with a separate License: tag
> for each one, that might work.  We already have some theme packages like that,
> e.g. gnome-theme-clearlooks-bigpack-0.6-6.fc7

Although that package is actually a set of themes, but you get the idea. 

Updated the package less the license with the spec file and splitting the source
into separate packages.

I'm assuming the docs are going to be the same as what they are now but
adjustments will be made if need to.

http://www.mwiriadi.id.au/fedora-spec/gnome-themes-extras

(In reply to comment #10)
> Updated the package less the license with the spec file and splitting the source
> into separate packages.

- The License tag in the main package "LGPL" is still wrong and each subpackage
should have it's own License tag if they are different, I think.

- Each of the subpackages should probably not have the same documentation files,
either, that should be included in the main package

- The main package doesn't currently have it's own %files section, which would
make it an empty package.

I'm actually beginning to think to sticking to a single monolithic package, and
including multiple tags in License to cover each one.

I should have clarified I'm still waiting for feedback from gnome so I have left
the files as is. I won't put the same docs in the same section when I have the
different docs.

While the license tag is incorrect I can't change it till gnome gets back to me.
 The package I have listed is just waiting for there alterations.

(Removing NEEDSPONSOR: bug 426733)

(In reply to comment #12)
> I should have clarified I'm still waiting for feedback from gnome so I have left
> the files as is. I won't put the same docs in the same section when I have the
> different docs.
> 
> While the license tag is incorrect I can't change it till gnome gets back to me.
>  The package I have listed is just waiting for there alterations.

Ping?  Is it possible make updates to this package now?  I'd like to finish up
the review so you can get this into Fedora.

I have pinged upstream 2-3 times with the same bug listed above.  They have
advised be that they will be fixing it however since the first bug request they
have not fixed it.

So I'm not to sure what else I can do to request upstream to include the
licensing information since they are not being to helpful at the moment.

(In reply to comment #15)

> So I'm not to sure what else I can do to request upstream to include the
> licensing information since they are not being to helpful at the moment.

Perhaps just package the theme(s) that do have licenses for the moment (and
strip out the packages that don't from the tarball) and you can add them back as
upstream confirms licenses.

Only COPYING files I can find is Gion and Neu so is it ok to remove it after
building or do I need to strip it out of the source? If I have to strip it out
of Source that would mean I would possibly need to patch the configure script?

Major issues with this package.

All the patches in http://mwiriadi.fedorapeople.org/packages/gnome-themes-extras/

This doesn't build.

make[2]: Entering directory
`/home/marc/rpmbuild/BUILD/gnome-themes-extras-2.20/icon-themes'
make[2]: *** No rule to make target `all'.  Stop.
make[2]: Leaving directory
`/home/marc/rpmbuild/BUILD/gnome-themes-extras-2.20/icon-themes'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/home/marc/rpmbuild/BUILD/gnome-themes-extras-2.20'
make: *** [all] Error 2
error: Bad exit status from /var/tmp/rpm-tmp.18227 (%build)

I didn't adjust any of the makefiles apart from the patches.  I did a visual
comparison of the two sections for make all and there is no difference apart
from what is patched.

Any suggestions?

Here is the SRPM without the patches, as discussed sadly it doesn't build with
the patches.

http://mwiriadi.fedorapeople.org/packages/gnome-themes-extras/gnome-themes-extras-2.20-2.fc8.src.rpm

Here's the upstream license clarification request bug:

http://bugzilla.gnome.org/show_bug.cgi?id=496096

It looks like we are go tentatively for GPLv2, but I'd like to like to check
whether they mean GPLv2+ (i.e. or later version), which is the most likely. 
Judging from some of the comments it seems that upstream doesn't really
understand the meaning of GPL versions very well.

I saw the posts I appreciate your input on this.

http://mwiriadi.fedorapeople.org/packages/gnome-themes-extras/gnome-themes-extras.spec

http://mwiriadi.fedorapeople.org/packages/gnome-themes-extras/gnome-themes-extras-2.20-3.fc8.src.rpm

MUST FIX items:

1) OK, I think that we should go back to the monolithic package for the moment,
to avoid duplication of docs and to make sure upgrades from the old
gnome-themes-extras goes smoothly.

The good news is that I did a koji build on rawhide that worked:

http://koji.fedoraproject.org/koji/taskinfo?taskID=336678

Also there are some (mostly) small issues with running rpmlint on the packages

$ rpmlint *.rpm

gnome-themes-extras.src: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab:
line 20)
gnome-themes-extras-darklooks.noarch: W: summary-not-capitalized darklooks theme
gnome-themes-extras-darklooks.noarch: E: description-line-too-long Darklooks is
a meta-theme that is part of the gnome-themes-extras package. This package
gnome-themes-extras-foxtrot.noarch: E: description-line-too-long Foxtrot is a
meta-theme that is part of the gnome-themes-extras package. This package
gnome-themes-extras-gion.noarch: E: description-line-too-long Gion is a
meta-theme that is part of the gnome-themes-extras package.  This package
gnome-themes-extras-neu.noarch: W: spelling-error-in-description pacakge package
gnome-themes-extras-neu.noarch: E: description-line-too-long Neu is a meta-theme
that is part of the gnome-themes-extras pacakge. This package

2) Remember, Description lines should be no longer than 80 chars, and that
Summary should start with a capital letter.

3) You are also mixing tabs and spaces in the spec file which makes the spec
file look odd in different editors, for example in Emacs it looks like this:

%package		foxtrot
Requires:		gnome-icon-theme, gnome-themes
Group:		User Interface/Desktops
Summary:		Foxtrot metatheme
%description	foxtrot
Foxtrot is a meta-theme that is part of the gnome-themes-extras package. This
package requires that you use a Gnome 2.2 release or newer.

4) * Mon Jan 7 2008 Marc Wiriadisastra <marc.au> - 2.20-3
- Added patches to remove non-licensed themes

isn't true any more, so should be removed/updated.

Also put a link to the upstream GNOME bugzilla link in a comment in the spec
file with a note saying upstream clarified the license on the bug and are
(hopefully) working to include it in the next release.  Please keep checking to
see whether they mean GPLv2+ (which they probably do) rather than GPLv2 and
update spec accordingly.

You watch to see they actually update SVN here:

http://svn.gnome.org/viewvc/gnome-themes-extras/trunk/

Done all of that I added the link to the gnome bug in the changelog thats the
only visual place that it is.

http://mwiriadi.fedorapeople.org/packages/gnome-themes-extras/gnome-themes-extras-2.20-4.fc8.src.rpm
http://mwiriadi.fedorapeople.org/packages/gnome-themes-extras/gnome-themes-extras.spec

All looks good, so this package is:

APPROVED

This package is in Fedora but ownership has been dropped I submitted this as a
new review originally because of the time and version numbers.

Should I just add myself as a maintainer and just do an update?  Do I have to
fill out the cvs form to add myself as a maintainer?

Package Change Request
======================
Package Name: gnome-themes-extras
Updated Fedora Owners: mwiriadi
New Branches: F-8 devel

Looks like it's still owned in devel... 

You can go to: 

https://admin.fedoraproject.org/pkgdb/packages/name/gnome-themes-extras

and take ownership of the F-8 branch and ask for ownership of the devel branch. 

(In reply to comment #29)
> Looks like it's still owned in devel... 
> 
> You can go to: 
> 
> https://admin.fedoraproject.org/pkgdb/packages/name/gnome-themes-extras
> 
> and take ownership of the F-8 branch and ask for ownership of the devel branch. 

But there isn't an F-8 branch there.

Matthias: would you be willing to orphan the devel branch so that Marc can
maintain the entire package?

F-8 branch added. 

I have added myself as part of the group members but Matthias automatically has
gotten ownership.

Just sent Matthias a private e-mail (which I Cc'ed you on) to ask him to release
the package.  Hopefully he'll be more likely to read that than
bugzilla/packagedb spam

I finally took the time to release the devel branch. Sorry about that, I was
really planning on reviving that package... but if someone else can do it *now*,
then that's even better :-)

It's ready now yes I can upload it now and release it for Fedora 8 and devel
though it's up to you whether you want to do that.

Thanks heaps for all the help it's been uploaded and release for F-8 stable
since it's a 'new' package.

Marking this as next release.