[Bug 280751] Review Request: qmmp - Qt-based multimedia player

[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: qmmp - Qt-based multimedia player


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





------- Additional Comments From j.w.r.degoede at hhs.nl  2007-11-17 10:20 EST -------
Here are the Must Fix and Should fix items resulting from a full review, notice
that I've also recycled some of the comments above, thanks to all involved for
those!

Must Fix:
---------
* buildroot does not match the buildroot mandated by the guidelines
* remove "* MPEG1 layer 1/2/3 support;" from %description
* BuildRequires line longer then 80 chars, please split this up in multiple
  lines each starting with BuildRequires:
* desktop-file-install line longer then 80 chars, please split this up in
  multiple lines.
* use %defattr(-,root,root,-)
* put %doc directly under %defattr
* drop %{?_smp_mflags}, if it fails on some systems it must be dropped, the fact
  that it happens to work on others is not relevant, we don't want a lottery, we
  want reproducable builds
* add: "Requires(post): /sbin/ldconfig" and "Requires(postun): /sbin/ldconfig"
* All these lines:
  %dir %{_libdir}/qmmp
  %dir %{_libdir}/qmmp/Input
  %dir %{_libdir}/qmmp/Output
  %{_libdir}/qmmp/Input/*.so
  %{_libdir}/qmmp/Output/*.so
  Can be written as just:
  %{_libdir}/qmmp
  Notice that you will still need:
  %{_libdir}/libqmmp.so
  Listed seperately
* Drop the second %defattr line


Possible improvements:
----------------------
* Use single quotes instead of double quotes around your sed scripts so that
  you do not have to use \ infront of " inside the scripts.



-- 
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.