[Bug 511107] Review Request: rhythmbox-equalizer - An Equalizer plugin for Rhythmbox

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


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


Mads Kiilerich <mads at kiilerich.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mads at kiilerich.com




--- Comment #1 from Mads Kiilerich <mads at kiilerich.com>  2009-07-17 20:09:50 EDT ---
The macro up_name ... It isn't obvious to me what its name is short for, so it
doesn't improve readability much. The value of the macro will probably never
change, and the name of the macro is longer than its value. I suggest dropping
it.

The source has no indication of the license. I also don't see any indication of
the license on upstream site, so I can't confirm the GPL+ license. Upstream
should be asked to state the license explicitly in the relased tar ball. And
Rhythmbox is GPLv2+, so I don't think a GPL+ plugin like this(?) is legal?

The URL points to a blog covering many topics. If no real site exists then we
should use for example a stable link to the announcement of this release.

ChangeLog and TODO ... The content of these files doesn't add much value to the
package. If they were included in the upstream package they probably shouldn't
be included in the package anyway. Supplying the files as extra sources without
any indication of the origin makes it even more questionable. If anything then
upstream should be asked to include the files in the tar-ball.

Rhythmbox already requires python, so requiring it here is not strictly
necessary, but I guess that stating it explicitly is fine. But then it should
also state all other requirements explicitly.

The source contains .pyc files - they are a kind of pre-built binaries and
should be removed in %prep.

only-non-binary-in-usr-lib is caused by the location of rhythmboxs extension
folder. There is nothing this package can do to fix it. BUT on x86 it uses
/usr/lib/rhythmbox/plugins/rbeq while it is /usr/lib64/rhythmbox/plugins/rbeq
on x86_64. So unfortunately this package isn't and can't be noarch.

I have successfully tested that the package works on x86.

--
[Looking for sponsor and review on bug 509936]

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