Bug 198944

Note that there's an existing compiz review ticket pending for Extras.  What
should happen to it?

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=192432

I'd propose to close it as obsolete once compiz shows up in core

How does this spec compare to the one in the Extras review?  Does this one have
the fixes and updates generated from that review?

NEEDSWORK:
- release string should be whole number unless it is a pre-release (snapshot). 
If it is a snapshot, as it looks like, you should use the prelease naming
scheme:
http://fedoraproject.org/wiki/Packaging/NamingGuidelines#head-d97a3f40b6dd9d2288206ac9bd8f1bf9b791b22a
- dist tag is %{?dist} not %{dist}
- gconf stuff should follow the scriptletsnippits or have good reason not to:
http://fedoraproject.org/wiki/ScriptletSnippets#head-ff64cd482595764f672082d5a3b83e1fc22962e8
- changelog shouldn't use dist tag as part of the version-release.
- missing br gettext

rpmlint output (once added gettext)
W: compiz-debuginfo invalid-license X11/MIT/GPL
W: compiz invalid-license X11/MIT/GPL
W: compiz invalid-license X11/MIT/GPL

Ignorable I suppose, but it would help if upstream included a LICENSE file.

W: compiz macro-in-%changelog dist

Leave out the macro.

W: compiz mixed-use-of-spaces-and-tabs

Ignorable

W: compiz non-conffile-in-etc /etc/gconf/schemas/compiz.schemas

Should this be marked a conf file?

- Can you please explain what version number you would like to see here ?
  compiz-0.0.13-5.1.fedora6
  sounds like a perfectly fine n-v-r to me

- The GConf invokations in %post an %preun look perfectly fine to me. 
  This insistance on slavishly copying some snipplets from a wiki page
  is really the ugliest part of package review...
  The one thing that is wrong with the gconf invokations is the --shutdown
  I think that has no place there.

- We never mark gconf schema files as conf files, why do we have to go thorough
  this again for every package review ?

I'll fix the other issues 

Spec: http://people.redhat.com/mclasen/review/compiz.spec
SRPM: http://people.redhat.com/mclasen/review/compiz-0.0.13-0.5.20070717git.src.rpm

Thanks for the review, Jesse, comments below.

(In reply to comment #3)
> How does this spec compare to the one in the Extras review?  Does this one have
> the fixes and updates generated from that review?

To the extent that it makes sense, yes.

> NEEDSWORK:
> - release string should be whole number unless it is a pre-release (snapshot). 
> If it is a snapshot, as it looks like, you should use the prelease naming
> scheme:
>
http://fedoraproject.org/wiki/Packaging/NamingGuidelines#head-d97a3f40b6dd9d2288206ac9bd8f1bf9b791b22a
> - dist tag is %{?dist} not %{dist}
> - gconf stuff should follow the scriptletsnippits or have good reason not to:
>
http://fedoraproject.org/wiki/ScriptletSnippets#head-ff64cd482595764f672082d5a3b83e1fc22962e8
> - changelog shouldn't use dist tag as part of the version-release.
> - missing br gettext

Fixed (thanks, Matthias).

> rpmlint output (once added gettext)
> W: compiz-debuginfo invalid-license X11/MIT/GPL
> W: compiz invalid-license X11/MIT/GPL
> W: compiz invalid-license X11/MIT/GPL
> 
> Ignorable I suppose, but it would help if upstream included a LICENSE file.

Upstream doesn't include a license because different parts of the project are
under different licenses.  You need to check individual files for their license.

> W: compiz macro-in-%changelog dist
> 
> Leave out the macro.
> 
> W: compiz mixed-use-of-spaces-and-tabs
> 
> Ignorable
> 
> W: compiz non-conffile-in-etc /etc/gconf/schemas/compiz.schemas
> 
> Should this be marked a conf file?

Changes look good, rpmlint is fine (with ignores in place)

Approving.  Who wants to own this package in dist-fc6, and how will we put it in
Comps?

package owner will be krh, I'll handle the initial import and comps addition

package added to dist.

Please close when built into rawhide.