[Bug 447477] Review Request: qgtkstyle - Qt style rendering using GTK+ themes

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: qgtkstyle - Qt style rendering using GTK+ themes


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





------- Additional Comments From kevin at tigcc.ticalc.org  2008-05-25 02:17 EST -------
MUST Items:
+ rpmlint output OK:
qgtkstyle.src: W: mixed-use-of-spaces-and-tabs (spaces: line 11, tab: line 21)
Harmless, trivial to fix, but not a blocker.
qgtkstyle.i386: W: no-documentation
There's no documentation to package, not even a COPYING file.
! not entirely named and versioned according to the Package Naming Guidelines:
  The naming guidelines state that the date of the checkout should be 
specified. So the alphatag should be 20080523svn or 20080523svn610, not just 
svn610.
+ spec file name matches base package name
+ Packaging Guidelines:
  + License GPLv2 OK, matches actual license
  + No known patent problems
  + No emulator, no firmware, no binary-only or prebuilt components
  + Complies with the FHS
  + proper changelog, tags, BuildRoot, Requires, BuildRequires, Summary, 
Description
  ! but please use qt4-devel and qt4-x11 for the BRs/Reqs so this also builds 
on F8 (the F9+ packages Provide the qt4-* names)
  + no non-UTF-8 characters
  + no documentation from upstream to include
  + RPM_OPT_FLAGS are used
  + debuginfo package is valid
  + no static libraries nor .la files
  + no duplicated system libraries
  + no rpaths (at least on i386)
  + no configuration files, so %config guideline doesn't apply
  + no init scripts, so init script guideline doesn't apply
  + no GUI executables, so no .desktop file needed
  + ... and thus no desktop-file-install needed either
  + no timestamp-clobbering file commands
  + _smp_mflags used
  + scriptlets are valid
  + not a web application, so web application guideline doesn't apply
  + no conflicts
+ complies with all the legal guidelines
+ no license in the checkout to include as %doc
* skipping "source matches upstream" test because this is a SVN checkout
! bad: .svn directory included in the tarball
+ builds on at least one arch (F10 i386 mock, tested by submitter)
+ no known non-working arches, so no ExcludeArch needed
+ no missing BuildRequires
+ no translations, so translation/locale guidelines don't apply
+ ldconfig correctly called in %post and %postun
+ package not relocatable
+ ownership correct (no package-specific directories which would have to be 
owned, doesn't own directories owned by another package)
+ no duplicate files in %files
+ permissions correct, defattr used correctly
+ %clean section present and correct
+ macros used where possible
+ no non-code content
+ no large documentation files, so no -doc package needed
+ no %doc files, so no %doc files required at runtime
+ no header files which would need to be in a -devel subpackage
+ no static libraries, so no -static package needed
+ no .pc files, so no Requires: pkgconfig needed
+ no devel symlinks which would need to be in a -devel subpackage
+ %{_qt4_plugindir}/styles/libgtkstyle.so plugin (NOT a symlinks) is correctly 
NOT in -devel
+ no -devel package, so "-devel should require %{name} = %{version}-%{release}" 
is irrelevant
+ no .la files
+ no GUI executables, so no .desktop file needed
+ buildroot is deleted at the beginning of %install
+ all filenames are valid UTF-8

SHOULD Items:
! upstream includes no COPYING file, they should, maybe try talking to them?
+ no translations for description and summary provided by upstream
+ package builds in mock (F10 i386, tested by submitter)
* not tested if the package builds on all architectures
* not tested functionality because I don't have Qt 4.4 yet
+ scriptlets are sane
+ no subpackages other than -devel, so "Usually, subpackages other than devel 
should require the base package using a fully versioned dependency." is 
irrelevant
+ no .pc files, so "placement of .pc files" is irrelevant
+ no file dependencies


MUST FIX:
* You have to use 20080523svn or 20080523svn610, not just svn610, for the 
alphatag.

SHOULD FIX:
* Please use qt4-devel and qt4-x11 for the BRs/Reqs (instead of qt-devel and 
qt-x11) so this also builds on F8 (once Qt 4.4 gets pushed there). The F9+ 
packages Provide the qt4-* names, so no conditionals are needed.
* Excluding the .svn directory from your tarball would be nice, though that's a 
bit of a nitpick.
* It would be nice if you could talk to upstream about 1. including a COPYING 
file and 2. using the standard Trolltech "GPLv2 or GPLv3" boilerplate instead 
of the old GPLv2 only they used (also given that their Google Code page claims 
GPLv3 as the license), but this isn't a requirement for inclusion.

Fix the one MUST FIX issue (and please also at least the first SHOULD FIX issue 
while you are at it) and I'll approve this.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.