Bug 191538

Hi Fernando.  Here are some comments.  Some of these may seem nit-picky - sorry
about that...

* Release should use %{?dist}

* Alignment is inconsistent with rest of Fedora, so..
Summary: Qt based Fluidsynth GUI front end
Name: qsynth
Version: 0.2.5
..should be...
Summary: Qt based Fluidsynth GUI front end
Name:    qsynth
Version: 0.2.5

* Some people prefer that we use %makeinstall, although I don't know if it's 
required.  I've been using it in all my spec files based on somebody's request.

* Don't install the generic INSTALL file.

* Remember to put version info for each changelog entry 
rpmlint ../RPMS/i386/qsynth-0.2.5-2.i386.rpm
W: qsynth no-version-in-last-changelog

* I believe you should use the following before %configure...
unset QTDIR || : ; . /etc/profile.d/qt.sh
export QTLIB=${QTDIR}/lib QTINC=${QTDIR}/include

That's it for now.  Thanks!

One more thing.. I tried this with VintageDreams soundfont, and found that I had
to turn the gain down to 10 or less in order to stop the output from red-lining.
 The gain setting defaults to 100.  Is this expected?

I'm reviewing fluidsynth now so that this one can get some progress too.

MUST:
=====
* rpmlint output is:
W: qsynth no-version-in-last-changelog
W: qsynth-debuginfo no-version-in-last-changelog
These both must be fixed.
* Package and spec file named appropriately
* Packaged according to packaging guidelines
* License (GPL) ok, license file included
* spec file is legible and in Am. English.
* Source matches upstream
* Compiles and builds on devel-i386
* BR: ok
* No locales
* No shared libraries
* Not relocatable
* Package owns / or requires all dirs
* No duplicate files & Permissions ok
* %clean & macro usage OK
* Contains code only
* %doc does not affect runtime, and isn't large enough to warrent a sub package
* no -devel package needed, no libs / .la files.
* .desktop file as required and properly installed

MUST fix:
=========
* rpmlint output is:
W: qsynth no-version-in-last-changelog
W: qsynth-debuginfo no-version-in-last-changelog
These both must be fixed.
* Source0: http://dl.sf.net/qsynth/qsynth-%{version}.tar.gz
 Must be: Source0: http://dl.sf.net/sourceforge/qsynth/qsynth-%{version}.tar.gz
* %makeinstall is broken (yeah I know, someone should fix it) please use:
  make install DESTDIR=$RPM_BUILD_ROOT instead. (Or make that %{__make} ......)
* Under %files I see %{_datadir}/icons/qsynth.png, that is not according to the
  freedesktop.org icon standard, it should go under:
  %{_datadir}/icons/hicolor/32x32/apps
  Where 32x32 is the size of the icon, please do ls /usr/share/icons/hicolor/
  to see the available valid sizes, if the icon doesn't match any pick the 
  closest.
* Once the icon is in the proper case you must add %post(un) script to update 
  the icon-cache see:
http://fedoraproject.org/wiki/ScriptletSnippets#head-fc74f078205565f961f6d836b77c3428619c689d

(In reply to comment #4)
> * rpmlint output is:
> W: qsynth no-version-in-last-changelog
> W: qsynth-debuginfo no-version-in-last-changelog
> These both must be fixed.
> * Source0: http://dl.sf.net/qsynth/qsynth-%{version}.tar.gz
>  Must be: Source0: http://dl.sf.net/sourceforge/qsynth/qsynth-%{version}.tar.gz
> * %makeinstall is broken (yeah I know, someone should fix it) please use:
>   make install DESTDIR=$RPM_BUILD_ROOT instead. (Or make that %{__make} ......)
> * Under %files I see %{_datadir}/icons/qsynth.png, that is not according to the
>   freedesktop.org icon standard, it should go under:
>   %{_datadir}/icons/hicolor/32x32/apps
>   Where 32x32 is the size of the icon, please do ls /usr/share/icons/hicolor/
>   to see the available valid sizes, if the icon doesn't match any pick the 
>   closest.
> * Once the icon is in the proper case you must add %post(un) script to update 
>   the icon-cache see:
>
http://fedoraproject.org/wiki/ScriptletSnippets#head-fc74f078205565f961f6d836b77c3428619c689d

All fixed:
Spec URL: http://ccrma.stanford.edu/planetccrma/extras/qsynth.spec
SRPM URL: http://ccrma.stanford.edu/planetccrma/extras/qsynth-0.2.5-4.src.rpm

Looks good, approved and so does qjackctl, concider yourself sponsored.

Please create an account (or add yourself to the cvsextras group if you alreayd
have). This and the next steps are described here:
http://fedoraproject.org/wiki/Extras/Contributors .

Notice that I'm keeping the blocker bug as FE-REVIEW for now and that you
shouldn't import this yet as not all deps (fluidsynth) have been imported and
build. I've done a full review of the deps, but unfortunatly no response sofar.

Erm.

Is it me or haven't you created an Fedora account yet. As far as I'm concerned
your good to go to create a Fedora account and then I'll sponsor you. After
which you can imprt qjackctl, this one will has to wait until al the deps are done.

(In reply to comment #7)
> Erm.
> 
> Is it me or haven't you created an Fedora account yet. As far as I'm concerned
> your good to go to create a Fedora account and then I'll sponsor you. 

Sorry, no, it is not you, I have had a very busy week... I'll get to it as soon
as I can find a minute. I really appreciate your sponsorship. 

(In reply to comment #8)
> Sorry, no, it is not you, I have had a very busy week... I'll get to it as soon
> as I can find a minute. I really appreciate your sponsorship. 
> 

No problem we all have those sometimes (busy weeks).

Oops, I forgot to set this to FE-ACCEPT, doing so now.