Bug 1355

The app doesn't have a icon when it is running and makes it difficult to spot it in a task list.  

This is package is not suitable for rpmfusion, since it also support gstreamer backend(actually upstream recommend this backend)


See http://flavio.tordini.org/minitube/minitube-linux-setup

Ok, I'll close this, as this has also been repoened in the fedora bugzilla, will revisit if needed.

This package is NOT suitable for Fedora. It DOES NOT WORK without packages from RPM Fusion, even with the GStreamer backend, since it will try to use an MP4 or FLV format, not WebM. In addition, the GStreamer backend for Phonon is NOT the default, and this is a systemwide setting and cannot be changed per application.

Therefore, RPM Fusion IS the correct place for this package.

IMHO the package should be in RPM Fusion Free with a hard Requires: xine-lib-extras-freeworld, that way it'll work out of the box.

http://magnu5.fedorapeople.org/review/minitube/SPECS/minitube.spec
http://magnu5.fedorapeople.org/review/minitube/SRPMS/minitube-1.1-2.fc13.src.rpm

rpmlint -iv minitube-1.1-2.fc13.src.rpm 
minitube.src: I: checking
minitube.src: I: checking-url http://flavio.tordini.org/minitube (timeout 10 seconds)
minitube.src: I: checking-url http://flavio.tordini.org/files/minitube/minitube-1.1.tar.gz (timeout 10 seconds)
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

rpmlint -iv minitube.spec 
minitube.spec: I: checking-url http://flavio.tordini.org/files/minitube/minitube-1.1.tar.gz (timeout 10 seconds)
0 packages and 1 specfiles checked; 0 errors, 0 warnings.

added requires as kevin suggested.

The fact is minitube can't play a lot of videos in youtube without codecs from rpmfusion(xine-lib-freeworld or gstreamer-plugins-ugly). But it should not depend on any packages in rpmfusion, because we can not force users to use xine backend. 

The gstreamer backend is the recommended backend by the minitube author, also phonon itself don't depend on any backends explicitly. Phonon only depends on a virtual provides phonon-backend.

rpm -qa|grep phonon
phonon-4.4.2-1.fc13.x86_64
phonon-backend-gstreamer-4.4.2-1.fc13.x86_64

(In reply to comment #2)
> This is package is not suitable for rpmfusion, since it also support gstreamer
> backend(actually upstream recommend this backend)
> 
> 
> See http://flavio.tordini.org/minitube/minitube-linux-setup
> 

It's already been declined for the official fedora repo's

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

(In reply to comment #5)
> http://magnu5.fedorapeople.org/review/minitube/SPECS/minitube.spec
> http://magnu5.fedorapeople.org/review/minitube/SRPMS/minitube-1.1-2.fc13.src.rpm
> 
> rpmlint -iv minitube-1.1-2.fc13.src.rpm 
> minitube.src: I: checking
> minitube.src: I: checking-url http://flavio.tordini.org/minitube (timeout 10
> seconds)
> minitube.src: I: checking-url
> http://flavio.tordini.org/files/minitube/minitube-1.1.tar.gz (timeout 10
> seconds)
> 1 packages and 0 specfiles checked; 0 errors, 0 warnings.
> 
> rpmlint -iv minitube.spec 
> minitube.spec: I: checking-url
> http://flavio.tordini.org/files/minitube/minitube-1.1.tar.gz (timeout 10
> seconds)
> 0 packages and 1 specfiles checked; 0 errors, 0 warnings.
> 
> added requires as kevin suggested.
> 

Take a look at my spec file as you aren't validating the desktop file and haven't included the necessary scriptlets.

http://fedoraforum.org/leigh123linux/review/minitube/1/minitube.spec 

(In reply to comment #7)
> > 
> > See http://flavio.tordini.org/minitube/minitube-linux-setup
> > 
> It's already been declined for the official fedora repo's
> https://bugzilla.redhat.com/show_bug.cgi?id=583978

Things are a bit different since youtube will ship webm videos in the forseeable future. Actually, adding this package to rpmfusion can't help end users at all, because it can't depends on needed external bits from rpmfusion(gstreamer-* xine-lib-extras-freeworld). So users who want to use minitube still need to install codecs from rpmfusion manually.

Only that unlike in Fedora, in RPMFusion we can add a "README.Fedora" advising users how to also install the needed codecs from RPMFusion.

(In reply to comment #10)
> Only that unlike in Fedora, in RPMFusion we can add a "README.Fedora" advising
> users how to also install the needed codecs from RPMFusion.

Maybe this is the only valid benefic for keeping minitube in rpmfusion until webm is widely used in youtube :) 

new builds:

http://magnu5.fedorapeople.org/review/minitube/SPECS/minitube.spec
http://magnu5.fedorapeople.org/review/minitube/SRPMS/minitube-1.1-3.fc13.src.rpm

rpmlint -iv minitube.spec
minitube.spec: I: checking-url http://flavio.tordini.org/files/minitube/minitube-1.1.tar.gz (timeout 10 seconds)
0 packages and 1 specfiles checked; 0 errors, 0 warnings.

rpmlint -iv minitube-1.1-3.fc13.src.rpm 
minitube.src: I: checking
minitube.src: I: checking-url http://flavio.tordini.org/minitube (timeout 10 seconds)
minitube.src: I: checking-url http://flavio.tordini.org/files/minitube/minitube-1.1.tar.gz (timeout 10 seconds)
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

Stole lots of stuff from leigh (thanks!)
Also removed the Req: on xine-lib-extras-freeworld

> The fact is minitube can't play a lot of videos in youtube without codecs from
> rpmfusion(xine-lib-freeworld or gstreamer-plugins-ugly). But it should not
> depend on any packages in rpmfusion, because we can not force users to use xine
> backend.

The Xine backend is the default Phonon backend in Fedora, as such, we should make the package work out of the box with that backend.

> The gstreamer backend is the recommended backend by the minitube author,

This is not true. The Minitube FAQ explicitly says that KDE users will prefer the Xine backend and doesn't recommend one backend over the other. And this is not Minitube's decision anyway, the default Phonon backend is a systemwide decision.

> also phonon itself don't depend on any backends explicitly. Phonon only depends
> on a virtual provides phonon-backend.

We can change this if that's your problem (and in fact we probably will once we have phonon-backend-vlc, either to force xine to keep it as the default, or to force vlc for people who are upgrading if we switch the default).

> Actually, adding this package to rpmfusion can't help end users at all, because
> it can't depends on needed external bits from rpmfusion(gstreamer-*
> xine-lib-extras-freeworld).

Of course it can (and IMHO, MUST) depend on xine-lib-extras-freeworld (and probably phonon-backend-xine too, just to make sure that xine-lib-extras-freeworld has an effect). Making the package work out of the box with the default Phonon backend should be our top priority.

> > Actually, adding this package to rpmfusion can't help end users at all, because
> > it can't depends on needed external bits from rpmfusion(gstreamer-*
> > xine-lib-extras-freeworld).
> 
> Of course it can (and IMHO, MUST) depend on xine-lib-extras-freeworld (and
> probably phonon-backend-xine too, just to make sure that
> xine-lib-extras-freeworld has an effect). Making the package work out of the
> box with the default Phonon backend should be our top priority.

I second that. If possible ( and by requiring the packages which provide the  codecs it is possible) the package should work by default. For a worst case scenario, see my previous comment, which should be amended as "we can add a "README.Fedora" file advising users how to also install the needed codecs from RPMFusion or add a short paragraph in the Description tag, telling users that the codecs installed by packages X and Y are also needed."

(In reply to comment #13)
> > The gstreamer backend is the recommended backend by the minitube author,
> 
> This is not true. The Minitube FAQ explicitly says that KDE users will prefer
> the Xine backend and doesn't recommend one backend over the other. And this is
> not Minitube's decision anyway, the default Phonon backend is a systemwide
> decision.
> 
> > also phonon itself don't depend on any backends explicitly. Phonon only depends
> > on a virtual provides phonon-backend.
> 
> We can change this if that's your problem (and in fact we probably will once we
> have phonon-backend-vlc, either to force xine to keep it as the default, or to
> force vlc for people who are upgrading if we switch the default).
> 
> > Actually, adding this package to rpmfusion can't help end users at all, because
> > it can't depends on needed external bits from rpmfusion(gstreamer-*
> > xine-lib-extras-freeworld).
> 
> Of course it can (and IMHO, MUST) depend on xine-lib-extras-freeworld (and
> probably phonon-backend-xine too, just to make sure that
> xine-lib-extras-freeworld has an effect). Making the package work out of the
> box with the default Phonon backend should be our top priority.
> 

If we can only use xine backend, then why we packaged the other phonon backend? Work out of box is a goal, but not means we should force everybody to use the same backend. Actually, most video players can't play all supported vedio format by default. I think adding something to description or adding a README.fedora will be more suitable. 


If you want to set xine as the default backend in KDE, I think the best way is adding it to the default comps KDE group, then other desktop users can continue to use other backend.


FYI, I'm not sure if minitube can readlly works with xine backend.

From the author's comment yesterday
"Flavio says:
August 3, 2010 at 7:45 am

@Dirty Sanchez you mean: will someone ever fix the Xine engine (or Xine itself) so that it works with YouTube videos?" 

> If we can only use xine backend, then why we packaged the other phonon backend?

You can USE another backend, you'll just have xine-lib-extras-freeworld (and thus xine-lib) dragged in as a dependency even if you don't use it. It's the tradeoff for making things work out of the box.

> FYI, I'm not sure if minitube can readlly works with xine backend.

Magnus tested it, it worked just fine.

(In reply to comment #16)
> > If we can only use xine backend, then why we packaged the other phonon backend?
> 
> You can USE another backend, you'll just have xine-lib-extras-freeworld (and
> thus xine-lib) dragged in as a dependency even if you don't use it. It's the
> tradeoff for making things work out of the box.
>

No, I can't. If I install both xine and gstreamer in gnome desktop, then phonon will force me to use xine backend. KDE4 users can switch between different phonon backends, but gnome users can't.

That's why the installation guideline for this package is:

1.sudo apt-get install libqt4-network phonon-backend-gstreamer gstreamer0.10-ffmpeg gstreamer0.10-plugins-bad
2.sudo apt-get remove phonon-backend-xine

See http://flavio.tordini.org/minitube/minitube-linux-setup

I think any packages should not depends on either phonon-backend-xine or phonon-backend-gstreamer, choice between multiple backends is an important feature for phonon.

> No, I can't. If I install both xine and gstreamer in gnome desktop, then phonon
> will force me to use xine backend. KDE4 users can switch between different
> phonon backends, but gnome users can't.

You can fire up System Settings in GNOME too.

(In reply to comment #18)
> > No, I can't. If I install both xine and gstreamer in gnome desktop, then phonon
> > will force me to use xine backend. KDE4 users can switch between different
> > phonon backends, but gnome users can't.
> 
> You can fire up System Settings in GNOME too.
> 

Still can't find a place to switch phonon backend like Computer Admin - Mutimedia in KDE4.

So I suggest not to hardcode dependency on specific phonon backend.

Also, the author don't recommend to depend on xine explictly.

See https://bugs.launchpad.net/ubuntu/+source/minitube/+bug/572657(the last reply)
https://bugs.launchpad.net/ubuntu/+source/minitube/+bug/559906(#20)

Maybe off-topic, qtconfig-qt4 seems can only configure gstreamer backend in gnome, even I only install phonon-backend-xine

You need to fire up the KDE System Settings (systemsettings from kdebase-workspace), not the gnome-control-center nor qtconfig-qt4.

In my testing (using a fedora 13 kde clean install) I have come to the conclusion that this package uses whatever backend is available (xine/gstreamer) it does not need to depend on any of those explicitly. However if you are using the xine backend, then xine-lib-extras-freeworld is required for the videos to play, and using gstreamer, gstreamer-plugins-bad and gstreamer-ffmpeg are both needed. I am setting a a gnome VM to test further how it behaves in gnome.

(In reply to comment #21)
> In my testing (using a fedora 13 kde clean install) I have come to the
> conclusion that this package uses whatever backend is available
> (xine/gstreamer) it does not need to depend on any of those explicitly. However
> if you are using the xine backend, then xine-lib-extras-freeworld is required
> for the videos to play, and using gstreamer, gstreamer-plugins-bad and
> gstreamer-ffmpeg are both needed. I am setting a a gnome VM to test further how
> it behaves in gnome.
> 

ehh, it does indeed pull in phonon, phonon-backend-xine.

(In reply to comment #22)
> (In reply to comment #21)
> > In my testing (using a fedora 13 kde clean install) I have come to the
> > conclusion that this package uses whatever backend is available
> > (xine/gstreamer) it does not need to depend on any of those explicitly. However
> > if you are using the xine backend, then xine-lib-extras-freeworld is required
> > for the videos to play, and using gstreamer, gstreamer-plugins-bad and
> > gstreamer-ffmpeg are both needed. I am setting a a gnome VM to test further how
> > it behaves in gnome.
> > 
> 
> ehh, it does indeed pull in phonon, phonon-backend-xine.
> 

and installing gstreamer-ffmpeg and gstreamer-plugins-bad doesn't help at all in gnome.

Just a crazy thought: if webm support for this is on the horizon then why not review the package in Fedora now and import it here if we agree that's the best place; then it can get moved to Fedora without a additional review once the webm support shows up

(In reply to comment #22)
> (In reply to comment #21)
> > In my testing (using a fedora 13 kde clean install) I have come to the
> > conclusion that this package uses whatever backend is available
> > (xine/gstreamer) it does not need to depend on any of those explicitly. However
> > if you are using the xine backend, then xine-lib-extras-freeworld is required
> > for the videos to play, and using gstreamer, gstreamer-plugins-bad and
> > gstreamer-ffmpeg are both needed. I am setting a a gnome VM to test further how
> > it behaves in gnome.
> > 
> ehh, it does indeed pull in phonon, phonon-backend-xine.

No, on my laptop(F14 gnome), it can plays x264 videos with phonon-backend-gstreamer, it seems the author of this software also uses gnome.

The packaging issue could be resolved with meta subpackages for gnome and kde to drag in the required deps.

Not really. The correct dependency relation is:
if (phonon-backend-xine installed)
  Requires: xine-lib-extras-freeworld
elseif (phonon-backend-gstreamer installed)
  Requires: gstreamer-plugins-ugly gstreamer-plugins-bad gstreamer-ffmpeg
else
  ???
endif
but this relation cannot be expressed with RPM dependencies.

The next best thing we can do is:
Requires: phonon-backend-xine xine-lib-extras-freeworld
which will make it work by default.

An alternative solution would be to:
Requires: xine-lib-extras-freeworld
Requires: gstreamer-plugins-ugly gstreamer-plugins-bad gstreamer-ffmpeg
then it will work no matter what Phonon backend is in use, but it will force installing both GStreamer and xine-lib with the respective plugins.

(Note that the second solution does NOT drag in phonon-backend-xine, only xine-lib-extras-freeworld and (through dependencies) xine-lib.)

The third solution is:
Requires: xine-lib-extras-freeworld
which would make it work out of the box with Phonon-Xine, and drag in 2 extraneous dependencies (xine-lib-extras-freeworld and xine-lib, but not phonon-backend-xine) for Phonon-GStreamer users.

Unfortunately, the perfect solution does not exist.

(In reply to comment #28)
> (Note that the second solution does NOT drag in phonon-backend-xine, only
> xine-lib-extras-freeworld and (through dependencies) xine-lib.)
> 

Are you sure about this? I just installed minitube after removing phonon-backend-xine and phonon-backend-gstreamer, and phonon-backend-xine, phonon and qt-x11 got installed as dependancys.

If you don't have any Phonon backend installed, yum will pick -xine over -gstreamer, and that's what we want it to do. But it is currently possible to explicitly install phonon-backend-gstreamer and get only that.

(In reply to comment #30)
> (In reply to comment #28)
> > (Note that the second solution does NOT drag in phonon-backend-xine, only
> > xine-lib-extras-freeworld and (through dependencies) xine-lib.)
> > 
> Are you sure about this? I just installed minitube after removing
> phonon-backend-xine and phonon-backend-gstreamer, and phonon-backend-xine,
> phonon and qt-x11 got installed as dependancys.


If you install phonon-backend-gstreamer, then you won't install phonon-backend-xine as a dependency to minitube.

Actually, I think it'll be better to remove all those extra dependencies. Because most KDE users already install xine-lib-extras-freeworld(for amaroK), most GNOME users already install gsteamer-plugins-*(for totem). A simple desciption in rpm will be more appropiate.

I think our goal should be to support WebM out of the box (read: without having to install extra packages from rpmfusion). Google appears to be interested in switching from flash videos to WebM and is pushing for the support in web browsers. My guess is that they'll soon have all videos converted to WebM so in theory minitube should be able to play all content without needing flash.

Unfortunately, looks like our default Phonon backend (xine) needs ffmpeg for WebM demuxing and vpx decoding. Gstreamer, however, supports WebM demuxing out of the box and can decode vpx through libvpx without having to rely on ffmpeg.

Kevin Kofler: For a short while phonon-backend-gstreamer was the default in rawhide, but it never made it into any releases. What were the problems with it? Are there possibly any plans on trying to make gstreamer the default backend again?

In any case, looks like minitube is currently unable to play any content without ffmpeg, which means it'll have to go into rpmfusion. However, if upstream fixes minitube to work with just with gstreamer-plugins-bad-free, I think it should be good to go into Fedora proper. I'd be willing to re-review it in Fedora in that case.

Is anybody here going to do the official rpmfusion review? If not, I can do that.

> %{_datadir}/%{name}/locale/*.qm
This results in unowned directories: %{_datadir}/%{name}/ and %{_datadir}/%{name}/locale/ are not traced by this rpm.
To fix that, use either:
%{_datadir}/%{name}/
or:
%dir %{_datadir}/%{name}/
%dir %{_datadir}/%{name}/locale/
%{_datadir}/%{name}/locale/*.qm

It would also be nice to use %{version} macro in Source0 URL so that you wouldn't have to update version in two separate places (Version: and Source0:) when you update the package.

%{_datadir}/%{name}/locale/*.qm is a violation of the packaging guidelines for translations (they won't properly get lang-tagged), that's what %find_lang --with-qt is for.

(Therefore, listing %{_datadir}/%{name}/ and/or %{_datadir}/%{name}/locale/ without %dir is also not valid.)

Please look at the current revisions of e.g. qt, qt-assistant-adp or qgis for how to do this properly. (There may be other offenders, those should be fixed too.)

The licensing is unclear. How did you determine that the package is "GPLv3+ and LGPLv2+"?

Some of the source files are (C) 2009 Nokia Corporation and have proper license headers; other have no headers and I couldn't find any readme.txt file or similar which would clarify authors' intent. Can you ask upstream to clarify [1] the licensing of the files which lack license headers and preferably add proper license headers to those files?

[1] https://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Clarification

New files:
http://magnu5.fedorapeople.org/review/minitube/SPECS/minitube.spec
http://magnu5.fedorapeople.org/review/minitube/SRPMS/minitube-1.1-4.fc13.src.rpm

rpmlint -iv minitube.spec
minitube.spec: I: checking-url http://flavio.tordini.org/files/minitube/minitube-1.1.tar.gz (timeout 10 seconds)
0 packages and 1 specfiles checked; 0 errors, 0 warnings.

rpmlint -iv minitube-1.1-4.fc13.src.rpm 
minitube.src: I: checking
minitube.src: I: checking-url http://flavio.tordini.org/minitube (timeout 10 seconds)
minitube.src: I: checking-url http://flavio.tordini.org/files/minitube/minitube-1.1.tar.gz (timeout 10 seconds)
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

Many thanks to Kalev Lambert for helping me with the licensing and patching for the whole day, you rock!

There are still probably some things that need fixing in this, namely the 

%lang(ar) %{_datadir}/%{name}/locale/ar.qm
%lang(es) %{_datadir}/%{name}/locale/es.qm
%lang(gl) %{_datadir}/%{name}/locale/gl.qm
%lang(lat) %{_datadir}/%{name}/locale/lat.qm
%lang(uk) %{_datadir}/%{name}/locale/uk.qm

that I stole from Tom Callaway:
https://bugzilla.redhat.com/show_bug.cgi?id=584866#c4

%find_lang skips these and they show up as unpackaged files otherwise.
Any clues?

(In reply to comment #37)
> New files:
> http://magnu5.fedorapeople.org/review/minitube/SPECS/minitube.spec
> http://magnu5.fedorapeople.org/review/minitube/SRPMS/minitube-1.1-4.fc13.src.rpm
> 
> rpmlint -iv minitube.spec
> minitube.spec: I: checking-url
> http://flavio.tordini.org/files/minitube/minitube-1.1.tar.gz (timeout 10
> seconds)
> 0 packages and 1 specfiles checked; 0 errors, 0 warnings.
> 
> rpmlint -iv minitube-1.1-4.fc13.src.rpm 
> minitube.src: I: checking
> minitube.src: I: checking-url http://flavio.tordini.org/minitube (timeout 10
> seconds)
> minitube.src: I: checking-url
> http://flavio.tordini.org/files/minitube/minitube-1.1.tar.gz (timeout 10
> seconds)
> 1 packages and 0 specfiles checked; 0 errors, 0 warnings.
> 
> Many thanks to Kalev Lambert for helping me with the licensing and patching for
> the whole day, you rock!
> 
> There are still probably some things that need fixing in this, namely the 
> 
> %lang(ar) %{_datadir}/%{name}/locale/ar.qm
> %lang(es) %{_datadir}/%{name}/locale/es.qm
> %lang(gl) %{_datadir}/%{name}/locale/gl.qm
> %lang(lat) %{_datadir}/%{name}/locale/lat.qm
> %lang(uk) %{_datadir}/%{name}/locale/uk.qm
> 
> that I stole from Tom Callaway:
> https://bugzilla.redhat.com/show_bug.cgi?id=584866#c4
> 
> %find_lang skips these and they show up as unpackaged files otherwise.
> Any clues?
> 
Kalev Lember, oops :)

%find_lang doesn't find those translations because they aren't of the expected name_language.qm form, they only have language.qm. :-(

Created attachment 462 [details]
lang.patch

(In reply to comment #39)
> %find_lang doesn't find those translations because they aren't of the expected
> name_language.qm form, they only have language.qm. :-(
> 

Would this be an acceptable way to fix the lang file issues (The morons have misnamed the Latvian lang file lat.ts, it should be lv.ts)

http://countrycode.org/latvia

I have also added minitube to the file name.


https://bugzilla.rpmfusion.org/attachment.cgi?id=462&action=edit


I renamed lat.ts in the spec file

http://leigh123linux.fedorapeople.org/pub/SPECS/minitube.spec

Uh, it's not the country code that's relevant here, it's the language code.

http://magnu5.fedorapeople.org/review/minitube/SPECS/minitube.spec
http://magnu5.fedorapeople.org/review/minitube/SRPMS/minitube-1.1-5.fc13.src.rpm

rpmlint -iv minitube.spec
minitube.spec: I: checking-url http://flavio.tordini.org/files/minitube/minitube-1.1.tar.gz (timeout 10 seconds)
minitube.spec: W: invalid-url Source0: http://flavio.tordini.org/files/minitube/minitube-1.1.tar.gz <urlopen error timed out>
The value should be a valid, public HTTP, HTTPS, or FTP URL.

0 packages and 1 specfiles checked; 0 errors, 1 warnings.

rpmlint -iv minitube-1.1-5.fc13.src.rpm 
minitube.src: I: checking
minitube.src: I: checking-url http://flavio.tordini.org/minitube (timeout 10 seconds)
minitube.src: W: invalid-url URL: http://flavio.tordini.org/minitube <urlopen error timed out>
The value should be a valid, public HTTP, HTTPS, or FTP URL.

minitube.src: I: checking-url http://flavio.tordini.org/files/minitube/minitube-1.1.tar.gz (timeout 10 seconds)
minitube.src: W: invalid-url Source0: http://flavio.tordini.org/files/minitube/minitube-1.1.tar.gz <urlopen error timed out>
The value should be a valid, public HTTP, HTTPS, or FTP URL.

1 packages and 0 specfiles checked; 0 errors, 2 warnings.

webpage is down for the moment.
Leigh's patch seems to have fixed the locale issue, yay! :-)

(In reply to comment #43)
> webpage is down for the moment.
> Leigh's patch seems to have fixed the locale issue, yay! :-)

That patch looks nice, props to Leigh. Magnus, can you submit the patch upstream so that it'll be included in the next release? I think the reasoning that it's needed for rpm's find_lang to work is good enough.

Some issues with the package:
Mustfix:
 - /usr/share/minitube/ and /usr/share/minitube/locale/ directories are
   unowned.

Nice to have:
 - can you sort the list of files with license info at the top of the
   spec file? It's a bit hard to read the way it is now.
 - file names like lang.patch and QString.patch have a good chance of
   clashing with files from other source packages (not in the official build
   system, but when people "rpm -i" the source rpms in their own computer).
   Can you prefix them with minitube- like minitube-lang.patch,
   minitube-QString.patch?
 - it might be safer to BR qt4-devel instead of qt-devel, but see below:

$ repoquery --whatprovides qt-devel
qt-devel-1:4.6.3-8.fc13.i686
qt-devel-1:4.6.3-8.fc13.x86_64
qt-devel-1:4.6.2-16.fc13.i686
qt3-devel-0:3.3.8b-29.fc13.x86_64
qt3-devel-0:3.3.8b-29.fc13.i686
qt-devel-1:4.6.2-16.fc13.x86_64
qt-devel-1:4.6.3-8.fc13.i686
qt-devel-1:4.6.3-8.fc13.x86_64

Kevin: Why do the qt3-devel packages Provide qt-devel? Is it really needed? Looks like a good way to shoot yourself in the foot.

You should always specifically BR qt4-devel, it's unambiguous and it will continue to work when we'll have Qt 5.

That said, I think the Provides: qt-devel is indeed not useful in qt3-devel.

(In reply to comment #45)
> That said, I think the Provides: qt-devel is indeed not useful in qt3-devel.

Opened a ticket: https://bugzilla.redhat.com/show_bug.cgi?id=623106

http://magnu5.fedorapeople.org/review/minitube/SPECS/minitube.spec
http://magnu5.fedorapeople.org/review/minitube/SRPMS/minitube-1.1-6.fc13.src.rpm

rpmlint -iv minitube.spec
minitube.spec: I: checking-url http://flavio.tordini.org/files/minitube/minitube-1.1.tar.gz (timeout 10 seconds)
0 packages and 1 specfiles checked; 0 errors, 0 warnings.

rpmlint -iv minitube-1.1-6.fc13.src.rpm 
minitube.src: I: checking
minitube.src: I: checking-url http://flavio.tordini.org/minitube (timeout 10 seconds)
minitube.src: I: checking-url http://flavio.tordini.org/files/minitube/minitube-1.1.tar.gz (timeout 10 seconds)
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

Sure, I'll send it upstream. Other issues mentioned are also fixed.

RPM Fusion review minitube-1.1-6.fc13.src.rpm 2010-08-11

+ OK
! needs attention

rpmlint output:
$ rpmlint minitube-1.1-6.fc13.src.rpm minitube minitube-debuginfo
minitube.i686: W: no-manual-page-for-binary minitube
3 packages and 0 specfiles checked; 0 errors, 1 warnings.

+ Rpmlint warnings are harmless and can be ignored
+ The package is named according to the Package Naming Guidelines
+ Spec file name matches the base package name
+ Package meets the Packaging Guidelines
+ The package is licensed with a Fedora approved license and meets the Licensing Guidelines.
+ The license field in the spec file matches the actual license
+ The package contains license files (COPYING, LICENSE.LGPL, and INSTALL)
+ Spec file is written in American English
+ Spec file is legible
+ Upstream sources match sources in the srpm. md5sum:
  075841322631486a8bb69575ec05f5ca  minitube-1.1.tar.gz
  075841322631486a8bb69575ec05f5ca  Download/minitube-1.1.tar.gz

+ The package builds in koji
n/a ExcludeArch bugs filed
+ BuildRequires look sane
+ The spec file handles locales properly
n/a ldconfig
+ Package does not bundle copies of system libraries
n/a Package isn't relocatable
+ Package owns all directories it creates
+ No duplicate files in %files
+ Permissions are properly set and %files has %defattr
+ Consistent use of macros
+ The package must contain code, or permissable content.
n/a Large documentation files should go in -doc subpackage
+ Files marked %doc don't affect the package
n/a Header files must go to -devel
n/a Static libraries should be in -static
n/a Library files that end in .so must go to -devel package
n/a -devel must require the fully versioned base
+ Package doesn't contain any libtool .la files
+ Proper .desktop file handling
+ Directory ownership sane
+ Filenames are valid UTF-8

Looks good. APPROVED

Package CVS request
======================
Package Name: minitube
Short Description: A YouTube desktop client
Owners: magnu5
Branches: F12, F13, F14, devel
InitialCC:
----------------------
License tag: free

Packages built for all branches.