Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=213193
Summary: Review Request: gaim-rhythmbox - Rhythmbox plugin for GAIM Product: Fedora Extras Version: devel Platform: All OS/Version: Linux Status: NEW Severity: normal Priority: normal Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: michel.salim@gmail.com QAContact: fedora-package-review@redhat.com
Spec URL: http://hircus.org/fedora/gaim-rhythmbox/gaim-rhythmbox.spec SRPM URL: http://hircus.org/fedora/gaim-rhythmbox/gaim-rhythmbox-2.0-0.1.beta3.src.rpm Description: The Gaim-Rhythmbox plugin will automatically update your Gaim user and status info with the currently playing music in Rhythmbox.
If the artist and title are known, it will also attempt to create a link to the song's lyrics by using Google's "I'm Feeling Lucky" feature.
Gaim-Rhythmbox will replace %rb in your user info and status message with the song information.
Note: this is a beta version, targeting the beta release of GAIM that comes with FC6. When approved it'll be FC6+ only.
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: gaim-rhythmbox - Rhythmbox plugin for GAIM
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=213193
------- Additional Comments From Jochen@herr-schmitt.de 2006-10-31 15:16 EST ------- Good: + Local build works fine. + No complaints for source rpm. + No complaints for Binary rpms. + No complaitns for install rpm. + Tarball in source rpm matches with upstream. + Package contains verbatin License. + Able to install and uninstall package localy.
Unfortunateley, I was not able to check out if the plugin itself work properly.
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: gaim-rhythmbox - Rhythmbox plugin for GAIM
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=213193
------- Additional Comments From michel.salim@gmail.com 2006-10-31 18:26 EST ------- No IM account to test it with?
From my (limited) testing it works perfectly with AIM (both in status and info).
With Google Talk it works, but there is an open tag <rb> that's inserted before the song name without a close tag.
Do let me know if you want to do a full review. Thanks!
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: gaim-rhythmbox - Rhythmbox plugin for GAIM
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=213193
wart@kobold.org changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |wart@kobold.org
------- Additional Comments From wart@kobold.org 2006-11-02 18:58 EST ------- You're missing a couple of build requirements:
BR: gtk2-devel dbus-glib-devel dbus-devel
I'll pick up the full review tonight if Jochen doesn't get to it earlier.
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: gaim-rhythmbox - Rhythmbox plugin for GAIM
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=213193
------- Additional Comments From michel.salim@gmail.com 2006-11-02 19:58 EST ------- Jochen's no longer CC'ed, so I guess you get to review it.
http://hircus.org/fedora/gaim-rhythmbox/gaim-rhythmbox-2.0-0.2.beta3.src.rpm http://hircus.org/fedora/gaim-rhythmbox/gaim-rhythmbox.spec
I'm creating a mock tree right now to test it further - ldd /usr/lib64/gaim/gaim-rhythmbox.so disturbingly claims that the .so file requires anything from Xinerama to libxml2. Is that normal?
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: gaim-rhythmbox - Rhythmbox plugin for GAIM
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=213193
wart@kobold.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED AssignedTo|nobody@fedoraproject.org |wart@kobold.org
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: gaim-rhythmbox - Rhythmbox plugin for GAIM
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=213193
------- Additional Comments From wart@kobold.org 2006-11-02 21:40 EST ------- GOOD ==== * rpmlint output clean * Package named appropriately * Source matches upstream: a9e836986dae7857b408120782264d5a gaim-rhythmbox-2.0beta3.tar.gz * Builds in mock on FC6-i386, FC6-x86_64, FC7-i386, FC7-x86_64 * GPL license ok, license file included * Spec file legible and in Am. English. * Runs without crashing. Seems to work as expected with my AIM account. * No missing BR: * No locales * Not relocatable * Not a gui app; no need for a .desktop file * No need to run ldconfig; .so files are application plugins that aren't part of the system linker path. * Directory ownership ok * No duplicate %files * No need for -doc or -devel subpackages
MUSTFIX ======= * Inconsistent use of the custom 'prever' macro. You only use it once in %prep, but not at all in Source0 or Release. Either use it in all 3 places, or not at all.
NOTES ===== * You could also include AUTHORS and README in %doc * There's no need to split each sentence in %description into a separate paragraph. It just adds unnecessary whitespace and doesn't make it any easier to read. * Send the configure patch upstream so that it can be included in the final release. * I wouldn't worry about the shared library dependencies in the .so file. If you run ldd on the gaim executable itself, you'll see an almost-identical list of dependencies.
Not much here. Just fix the use of the prever macro and you're good to go.
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: gaim-rhythmbox - Rhythmbox plugin for GAIM
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=213193
------- Additional Comments From michel.salim@gmail.com 2006-11-02 22:16 EST ------- http://hircus.org/fedora/gaim-rhythmbox/gaim-rhythmbox-2.0-0.3.beta3.src.rpm http://hircus.org/fedora/gaim-rhythmbox/gaim-rhythmbox.spec
Fixed. Sorry about %description, I copied it from upstream and did not think to reformat it.
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: gaim-rhythmbox - Rhythmbox plugin for GAIM
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=213193
wart@kobold.org changed:
What |Removed |Added ---------------------------------------------------------------------------- OtherBugsDependingO|163776 |163779 nThis| |
------- Additional Comments From wart@kobold.org 2006-11-02 22:38 EST ------- Changes look good.
APPROVED
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: gaim-rhythmbox - Rhythmbox plugin for GAIM
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=213193
michel.salim@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution| |NEXTRELEASE
------- Additional Comments From michel.salim@gmail.com 2006-11-04 00:00 EST ------- Thanks! Actually, we missed a Require: on gaim. I'll put it in the next time there's an update, since it should only get triggered if someone 'yum remove' gaim and gaim-rhythmbox does not get removed as well.
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: gaim-rhythmbox - Rhythmbox plugin for GAIM
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=213193
------- Additional Comments From wart@kobold.org 2006-11-04 01:21 EST ------- The Requires: on gaim is picked up automatically:
$ rpm -q --requires gaim-rhythmbox | grep gaim libgaim.so.0()(64bit) $ rpm -q --whatprovides 'libgaim.so.0()(64bit)' gaim-2.0.0-0.17.beta4.fc6.x86_64
I also verified this by running 'yum remove gaim' and saw that it would remove gaim-rhythmbox, too.
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: gaim-rhythmbox - Rhythmbox plugin for GAIM
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=213193
------- Additional Comments From michel.salim@gmail.com 2006-11-04 11:08 EST ------- Bizarre, that does not appear on my system when using mock to build the package. When /not/ using mock the dependency appears properly.. I'll wait for the FE result to comes out and reverify.
% rpm -q gaim-rhythmbox gaim-rhythmbox-2.0-0.4.beta4.fc6
% rpm -q --requires gaim-rhythmbox | grep gaim %
package-review@lists.fedoraproject.org
-
Red Hat Bugzilla