Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: knights - A chess board for KDE
https://bugzilla.redhat.com/show_bug.cgi?id=674180
Summary: Review Request: knights - A chess board for KDE Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: medium Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: julian.fedora@googlemail.com QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, fedora-package-review@redhat.com Estimated Hours: 0.0 Classification: Fedora
Spec URL: http://julian.fedorapeople.org/knights/knights.spec SRPM URL: http://julian.fedorapeople.org/knights/knights-2.2.0-1.fc14.src.rpm Description: Knights is a chess board for KDE that supports playing against computer engines that support the XBoard protocol like GNUChess and also multiplayer games over the internet on FICS. It features automatic rule checking, themes, and nice animations
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=674180
Golo Fuchert packages@golotop.de changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |packages@golotop.de AssignedTo|nobody@fedoraproject.org |packages@golotop.de
--- Comment #1 from Golo Fuchert packages@golotop.de 2011-02-05 18:09:02 EST --- Knights should really be included in Fedora! I will do the review.
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=674180
--- Comment #2 from Golo Fuchert packages@golotop.de 2011-02-06 18:09:35 EST --- Here are some initial comments/questions before the review.
- I think the homepage in your spec file is wrong. It seems that there was a rewrite of Knights and your sources are provided by http://opendesktop.org/content/show.php?content=122046 - You might want to consider to use macros like %{name} in the Source0 tag, too. This will save you some work later on! - The headers of the source files state: "GNU General Public License [...]; either version 2 of the License or (at your option) version 3 or any later version accepted by the membership of KDE e.V." so GPLv3+ is somehow an oversimplification, isn't it? - Concerning Licensing: The source file contains no file containing the license text (e.g. COPYING), it is best to ask upstream to change this. http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text - Why are you not using make %{?_smp_mflags} ? Did I miss something?
Beside of that I think the package is ready for the review.
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=674180
--- Comment #3 from Julian Aloofi julian.fedora@googlemail.com 2011-02-07 07:50:22 EST --- (In reply to comment #2)
Here are some initial comments/questions before the review.
Thanks for reviewing it!
- I think the homepage in your spec file is wrong. It seems that there was a rewrite of Knights and your sources are provided by
kde-apps.org is a service run by opendesktop.org, so this is basically the same page as far as I understand.
- You might want to consider to use macros like %{name} in the Source0 tag, too. This will save you some work later on!
Good suggestion.
- The headers of the source files state: "GNU General Public License [...]; either version 2 of the License or (at your option) version 3 or any later version accepted by the membership of KDE e.V." so GPLv3+ is somehow an oversimplification, isn't it?
Probably yes. I'll try and find other applications with these licensing terms in Fedora and see what they did, if I can't find any I'll send a mail to Fedora legal.
- Concerning Licensing: The source file contains no file containing the license text (e.g. COPYING), it is best to ask upstream to change this. http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text
I'll tell upstream about it, missed that :)
- Why are you not using make %{?_smp_mflags} ? Did I miss something?
Oh, they should be there of course. I'll add them.
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=674180
--- Comment #4 from Julian Aloofi julian.fedora@googlemail.com 2011-02-07 09:09:27 EST ---
- The headers of the source files state: "GNU General Public License [...]; either version 2 of the License or (at your option) version 3 or any later version accepted by the membership of KDE e.V." so GPLv3+ is somehow an oversimplification, isn't it?
Probably yes. I'll try and find other applications with these licensing terms in Fedora and see what they did, if I can't find any I'll send a mail to Fedora legal.
I've spotted the license in Yakuake as well, and it doesn't have any special stuff in the "License:" field, so I guess this is OK.
Here are the latest packages so far:
Spec URL: http://julian.fedorapeople.org/knights/knights.spec SRPM URL: http://julian.fedorapeople.org/knights/knights-2.2.0-2.fc14.src.rpm
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=674180
Martin Gieseking martin.gieseking@uos.de changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.gieseking@uos.de
--- Comment #5 from Martin Gieseking martin.gieseking@uos.de 2011-02-07 13:11:46 EST --- I'd say the license is "GPLv2 or GPLv3" (plus a short comment about the mentioned restriction regarding later versions). GPLv3+ doesn't fit here.
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=674180
--- Comment #6 from Julian Aloofi julian.fedora@googlemail.com 2011-02-07 14:10:06 EST --- (In reply to comment #5)
I'd say the license is "GPLv2 or GPLv3" (plus a short comment about the mentioned restriction regarding later versions). GPLv3+ doesn't fit here.
I would just change it to GPLv2+ now. As pointed out, Yakuake does that and has the same licensing terms. Or is there so much uncertainty that a mail to legal@lists.fpo would be preferred?
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=674180
--- Comment #7 from Martin Gieseking martin.gieseking@uos.de 2011-02-07 14:42:38 EST --- To my limited legal knowledge, GPLv2+ is not quite correct here because of the given restriction. According to the copyright notice, only GPLv2 and GPLv3 have been approved while potential future GPL versions are still to be confirmed by the KDE team. GPLv2+ doesn't reflect this. But again, I'm not a legal expert. Maybe a question on the legal mailing list can shed some light on this.
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=674180
--- Comment #8 from Julian Aloofi julian.fedora@googlemail.com 2011-02-07 14:54:24 EST --- I'll send them a mail now.
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=674180
--- Comment #9 from Golo Fuchert packages@golotop.de 2011-02-07 15:16:33 EST --- I agree with Martin on this. I think this should be clarified, especially since it seems to affect other packages (like Yakuake) as well. Maybe after clarification this is an issue for the Fedora-packaging mailing list?
Concerning the homepage, you are absolutely right. I somehow ended up on the old homepage (http://kde-apps.org/content/show.php?content=20534) and was confused. Sorry for that.
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=674180
--- Comment #10 from Julian Aloofi julian.fedora@googlemail.com 2011-02-07 18:00:10 EST --- I got a reply from Fedora legal, and the appropriate License would be "GPLv2 and GPLv3" with a comment pointing out that the KDE e.V. might change this in the future. So here are the new packages:
Spec URL: http://julian.fedorapeople.org/knights/knights.spec SRPM URL: http://julian.fedorapeople.org/knights/knights-2.2.0-3.fc14.src.rpm
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=674180
--- Comment #11 from Martin Gieseking martin.gieseking@uos.de 2011-02-08 02:06:09 EST --- (In reply to comment #10)
"GPLv2 and GPLv3" with a comment pointing out that the KDE e.V. might change this in the future.
I think you meant "GPLv2 or GPLv3". "and" would be rather odd here.
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=674180
--- Comment #12 from Golo Fuchert packages@golotop.de 2011-02-09 17:22:36 EST --- Julian,
the package is almost ready, only the permissions for one file needs to be corrected.
$ rpmlint SPECS/knights.spec SRPMS/knights-2.2.0-3.fc14.src.rpm RPMS/i686/knights-2.2.0-3.fc14.i686.rpm knights.src: W: spelling-error %description -l en_US multiplayer -> multilayer, multiplexer, multiplier knights.i686: W: spelling-error %description -l en_US multiplayer -> multilayer, multiplexer, multiplier knights.i686: E: script-without-shebang /usr/share/applications/kde4/knights.desktop knights.i686: W: no-manual-page-for-binary knights 2 packages and 1 specfiles checked; 1 errors, 3 warnings.
Spelling errors are false positives, the .desktop file seems to have wrong permissions (755).
--------------------------------- key:
[+] OK [.] OK, not applicable [X] needs work ---------------------------------
[+] MUST: The package must be named according to the Package Naming Guidelines. [+] MUST: The spec file name must match the base package %{name}. [+] MUST: The package must meet the Packaging Guidelines. [+] MUST: The package must be licensed with a Fedora approved license. - GPLv2 or GPLv3 (with the possibility for an extension by KDE e.V.) [+] MUST: The License field in the package spec file must match the actual license. [.] MUST: The file containing the text of the license(s) for the package must be included in %doc. [+] MUST: The spec file must be written in American English. [+] MUST: The spec file for the package MUST be legible. [+] MUST: The sources used to build the package must match the upstream source. $ md5sum knights-2.2.0.tar.bz2{,.packaged} 6196e8ef8d2e7c16e38307469708f7cf knights-2.2.0.tar.bz2 6196e8ef8d2e7c16e38307469708f7cf knights-2.2.0.tar.bz2.packaged
[+] MUST: The package MUST successfully compile and build into binary rpms on at least one primary architecture. [.] MUST: If the package does not successfully compile, build or work on an architecture, ... [+] MUST: All build dependencies must be listed in BuildRequires. [+] MUST: When compiling C, C++, or Fortran files, %{optflags} must be applied. - implicitly by %cmake_kde4 [+] MUST: The spec file MUST handle locales properly. [+] MUST: If a package installs files below %{_datadir}/icons, the icon cache must be updated. [.] MUST: Packages storing shared library files (not just symlinks) must call ldconfig in %post and %postun. [+] MUST: Packages must NOT bundle copies of system libraries. [.] MUST: If the package is designed to be relocatable, ... [+] MUST: A package must own all directories that it creates. [+] MUST: A Fedora package must not list a file more than once in %files. [X] MUST: Permissions on files must be set properly. - see the rpmlint output above [+] MUST: Packages must not provide RPM dependency information when that information is not global in nature, or are otherwise handled. [.] MUST: When filtering automatically generated RPM dependency information, the filtering system implemented by Fedora must be used. [+] MUST: Each package must consistently use macros. [+] MUST: The package must contain code, or permissable content. [.] MUST: Large documentation files must go in a -doc subpackage. [+] MUST: Files in %doc must not affect the runtime of the application. [.] MUST: Header files must be in a -devel package. [.] MUST: Static libraries must be in a -static package. [.] MUST: If a package contains library files with a suffix (e.g. libfoo.so.1.1), ... [.] MUST: devel packages must require the base package using a fully versioned dependency. [+] MUST: Packages must NOT contain any .la libtool archives. [+] MUST: Packages containing GUI applications must include a %{name}.desktop file [+] MUST: .desktop files must be properly installed with desktop-file-install in the %install section. [+] MUST: Packages must not own files or directories already owned by other packages. [+] MUST: All filenames in rpm packages must be valid UTF-8.
[+] SHOULD: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream... - has been/will be done by the packager. [+] SHOULD: Timestamps of files should be preserved. [+] SHOULD: The reviewer should test that the package builds in mock. [+] SHOULD: The reviewer should test that the package functions as described. - Almost, the description didn't warn the reviewer to lose against gnuchess. ;-) [+] SHOULD: If scriptlets are used, those scriptlets must be sane. [.] SHOULD: Usually, subpackages other than devel should require the base package using a fully versioned dependency. [.] SHOULD: pkgconfig(.pc) files should be placed in a -devel pkg. [.] SHOULD: If the package has file dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin ... [X] SHOULD: Your package should contain man pages for binaries/scripts. - Maybe a plan for the future?
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=674180
--- Comment #13 from Julian Aloofi julian.fedora@googlemail.com 2011-02-12 20:00:36 EST --- Here are the new packages with fixed .desktop file permissions:
Spec URL: http://julian.fedorapeople.org/knights/knights.spec SRPM URL: http://julian.fedorapeople.org/knights/knights-2.2.0-4.fc14.src.rpm
(In reply to comment #12)
[+] SHOULD: The reviewer should test that the package functions as described. - Almost, the description didn't warn the reviewer to lose against gnuchess. ;-)
I'm glad I'm not the only one who can reproduce that problem ;)
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=674180
--- Comment #14 from Golo Fuchert packages@golotop.de 2011-02-13 05:44:08 EST --- All right, no further objections.
---------------- Package APPROVED ----------------
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=674180
Golo Fuchert packages@golotop.de changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-review+
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=674180
Julian Aloofi julian.fedora@googlemail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
--- Comment #15 from Julian Aloofi julian.fedora@googlemail.com 2011-02-13 14:50:50 EST --- New Package SCM Request ======================= Package Name: knights Short Description: A chess board for KDE Owners: julian Branches: f13 f14 InitialCC:
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=674180
--- Comment #16 from Julian Aloofi julian.fedora@googlemail.com 2011-02-13 14:51:47 EST --- (In reply to comment #14)
All right, no further objections.
Package APPROVED
Thanks for the review! :) If I can return the favour some day, just send me a mail ;)
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=674180
--- Comment #17 from Jason Tibbitts tibbs@math.uh.edu 2011-02-15 14:24:12 EST --- I also added an f15 branch.
Git done (by process-git-requests).
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=674180
Julian Aloofi julian.fedora@googlemail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution| |NEXTRELEASE Last Closed| |2011-02-15 14:41:51
--- Comment #18 from Julian Aloofi julian.fedora@googlemail.com 2011-02-15 14:41:51 EST --- (In reply to comment #17)
I also added an f15 branch.
Thanks, totally forgot that it's branched already! :)
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=674180
--- Comment #19 from Fedora Update System updates@fedoraproject.org 2011-02-15 15:39:18 EST --- knights-2.2.0-4.fc15 has been submitted as an update for Fedora 15. https://admin.fedoraproject.org/updates/knights-2.2.0-4.fc15
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=674180
--- Comment #20 from Fedora Update System updates@fedoraproject.org 2011-02-15 15:41:34 EST --- knights-2.2.0-4.fc14 has been submitted as an update for Fedora 14. https://admin.fedoraproject.org/updates/knights-2.2.0-4.fc14
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=674180
--- Comment #21 from Fedora Update System updates@fedoraproject.org 2011-02-15 15:43:28 EST --- knights-2.2.0-4.fc13 has been submitted as an update for Fedora 13. https://admin.fedoraproject.org/updates/knights-2.2.0-4.fc13
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=674180
--- Comment #22 from Fedora Update System updates@fedoraproject.org 2011-03-05 17:56:58 EST --- knights-2.2.0-4.fc14 has been pushed to the Fedora 14 stable repository. If problems still persist, please make note of it in this bug report.
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=674180
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Fixed In Version| |knights-2.2.0-4.fc14 Resolution|NEXTRELEASE |ERRATA
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=674180
--- Comment #23 from Fedora Update System updates@fedoraproject.org 2011-03-05 18:02:47 EST --- knights-2.2.0-4.fc13 has been pushed to the Fedora 13 stable repository. If problems still persist, please make note of it in this bug report.
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=674180
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Fixed In Version|knights-2.2.0-4.fc14 |knights-2.2.0-4.fc13
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=674180
--- Comment #24 from Fedora Update System updates@fedoraproject.org 2011-03-05 22:40:45 EST --- knights-2.2.0-4.fc15 has been pushed to the Fedora 15 stable repository. If problems still persist, please make note of it in this bug report.
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=674180
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Fixed In Version|knights-2.2.0-4.fc13 |knights-2.2.0-4.fc15
package-review@lists.fedoraproject.org