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@xxxxxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mads@xxxxxxxxxxxxx --- Comment #1 from Mads Kiilerich <mads@xxxxxxxxxxxxx> 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. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review