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
Knights should really be included in Fedora! I will do the review.
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.
(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 > http://opendesktop.org/content/show.php?content=122046 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.
> > - 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
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.
(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 would be preferred?
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.
I'll send them a mail now.
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.
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
(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.
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?
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 ;)
All right, no further objections. ---------------- Package APPROVED ----------------
New Package SCM Request ======================= Package Name: knights Short Description: A chess board for KDE Owners: julian Branches: f13 f14 InitialCC:
(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 ;)
I also added an f15 branch. Git done (by process-git-requests).
(In reply to comment #17) > I also added an f15 branch. Thanks, totally forgot that it's branched already! :)
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
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
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
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.
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.
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.