Spec URL: http://people.redhat.com/krh/compiz/compiz.spec SRPM URL: http://people.redhat.com/krh/compiz/compiz-0.0.13-5.1.fc6.src.rpm Description: compiz - open gl windows and compositing manager with patches to run on aiglx in fedora core.
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.