[Bug 1294523] Review Request: purple-skypeweb - Adds support for Skype to libpurple-based clients

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



--- Comment #4 from Simone Caronni <negativo17@xxxxxxxxx> ---
> - Sources used to build the package match the upstream source, as provided
>   in the spec URL.
>   Note: Upstream MD5sum check error, diff is in
>   /home/slaanesh/Downloads/1294523-purple-skypeweb/diff.txt
>   See: http://fedoraproject.org/wiki/Packaging/SourceURL

This is ok, the tarball is renamed according to packaging guidelines.

> - All build dependencies are listed in BuildRequires, except for any that
>   are listed in the exceptions section of Packaging Guidelines.
>   Note: These BR are not needed: gcc
>   See: http://fedoraproject.org/wiki/Packaging/Guidelines#Exceptions_2

This is ok as no longer true, BR can now be excplicitly required.

> [!]: License field in the package spec file matches the actual license.
>      Note: Checking patched sources after %prep for licenses. Licenses
>      found: "*No copyright* GPL (v3 or later)", "GPL (v3 or later)",
>      "Unknown or generated". 18 files have unknown license. Detailed output
>      of licensecheck in /home/slaanesh/Downloads/1294523-purple-
>      skypeweb/licensecheck.txt

This is ok, but please ask upstream to add all proper headers. Otherwise, since
this is a subset of the whole (obsolete) skype4pidgin tarball, just package in
the tarball only the required folders. You can use this guideline if you want
to proceed that way:

http://fedoraproject.org/wiki/Packaging:SourceURL#When_Upstream_uses_Prohibited_Code

> [!]: Package must own all directories that it creates.
>      Note: Directories without known owners:
>      /usr/share/pixmaps/pidgin/emotes/skype

Just remove the "theme" at the end of this line in the files section:

%{_datadir}/pixmaps/pidgin/emotes/skype/theme

> [!]: Patches link to upstream bugs/comments/lists or are otherwise
>      justified.

Please add some notes to the patch. I don't see why the code in the patch
should be removed, if there is an explanation for it, please add it to the SPEC
file.

Also, please remove the "%if 0%{?fedora}/%endif" part around Patch0. If I'm
downloading the source rpm for some rebuild on (whatever) unreleased
distribution (let's say a beta RHEL that has everything included) I won't get
the patch file from the Koji build, and I would need to download it from the
SCM.

It's perfectly acceptable to have patches shipped and just applied
conditionally.

> pidgin-skypeweb.noarch: W: spelling-error %description -l en_US inplemented -> implemented, supplemented, complemented

There's a typo in the comment, 's/inplemented/implemented/g'.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://lists.fedoraproject.org/admin/lists/package-review@xxxxxxxxxxxxxxxxxxxxxxx




[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]