Bug 910504 (kigo)

Summary: Review Request: kigo - Go Board game
Product: [Fedora] Fedora Reporter: Rex Dieter <rdieter>
Component: Package ReviewAssignee: nucleo <alekcejk>
Status: CLOSED RAWHIDE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: alekcejk, kevin, ltinkl, notting, package-review, than
Target Milestone: ---Flags: alekcejk: fedora‑review+
dennis: fedora‑cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2013-02-21 16:34:25 EST Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
Bug Depends On: 908920    
Bug Blocks: 656997, 907993    

Description Rex Dieter 2013-02-12 13:32:24 EST
Spec URL: http://rdieter.fedorapeople.org/rpms/kdegames/kigo.spec
SRPM URL: http://rdieter.fedorapeople.org/rpms/kdegames/kigo-4.10.0-2.fc18.src.rpm
Description: An implementation of the popular Go game
Fedora Account System Username: rdieter
Comment 1 nucleo 2013-02-19 15:43:30 EST
Summary from desktop file:

Summary: Go Board Game

Code under GPLv2 or GPLv3 license, docs under GFDL.

    This program is free software; you can redistribute it and/or
    modify it under the terms of the GNU General Public License as
    published by the Free Software Foundation; either version 2 of
    the License or (at your option) version 3 or any later version
    accepted by the membership of KDE e.V. (or its successor approved
    by the membership of KDE e.V.), which shall act as a proxy
    defined in Section 14 of version 3 of the license.

# KDE e.V. may determine that future GPL versions are accepted
License: (GPLv2 or GPLv3) and GFDL

COPYING.DOC should be added in %doc.

%description from documentation:

Go is a strategic board game for two players. It is also known as
igo (Japanese), weiqi or wei ch'i (Chinese) or baduk (Korean).
Go is noted for being rich in strategic complexity despite its
simple rules.
The game is played by two players who alternately place black and
white stones (playing pieces, now usually made of glass or plastic)
on the vacant intersections of a grid of 19×19 lines (9x9 or 13x13
lines for easier flavors).

Requires: gnugo

should be:

Requires: gnugo%{?_isa}

rpmlint output:

$ rpmlint kigo-4.10.0-2.fc18.i686.rpm kigo-debuginfo-4.10.0-2.fc18.i686.rpm kigo-4.10.0-2.fc18.src.rpm kigo.spec 
kigo.i686: W: no-manual-page-for-binary kigo
kigo.src:73: W: macro-in-comment %{name}
kigo.spec:73: W: macro-in-comment %{name}
3 packages and 1 specfiles checked; 0 errors, 3 warnings.

Commented line should be removed.
Comment 2 Kevin Kofler 2013-02-19 16:12:35 EST
> Requires: gnugo
> should be:
> Requires: gnugo%{?_isa}

No, why? It just invokes the binary, so why should there be a need to ISA-lock the dependency?
Comment 4 Rex Dieter 2013-02-19 16:17:14 EST
I agree with kevin, we shouldn't need to worry about multilib'ness of gnugo dep.

Spec URL: http://rdieter.fedorapeople.org/rpms/kdegames/kigo.spec
SRPM URL: http://rdieter.fedorapeople.org/rpms/kdegames/kigo-4.10.0-3.fc18.src.rpm

* Tue Feb 19 2013 Rex Dieter <rdieter@fedoraproject.org> 4.10.0-3
- update license, summary, description
Comment 5 Kevin Kofler 2013-02-19 16:22:09 EST
> https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#Requires

That only says HOW to make the dep arch-specific, it doesn't say WHEN to do it, and here it doesn't make sense. :-)
Comment 6 nucleo 2013-02-19 16:49:58 EST
MUST Items:
+ rpmlint output
  $ rpmlint kigo-4.10.0-2.fc18.i686.rpm kigo-debuginfo-4.10.0-2.fc18.i686.rpm kigo-4.10.0-3.fc18.src.rpm kigo.spec 
    kigo.i686: W: no-manual-page-for-binary kigo
    kigo.src: W: spelling-error %description -l en_US igo -> go, ego, ago
    kigo.src: W: spelling-error %description -l en_US weiqi -> Wei
    kigo.src: W: spelling-error %description -l en_US wei -> weigh, Wei, we
    kigo.src: W: spelling-error %description -l en_US ch'i -> chi, Ch'in
    kigo.src: W: spelling-error %description -l en_US baduk -> bad
    3 packages and 1 specfiles checked; 0 errors, 6 warnings.
+ named and versioned according to the Package Naming Guidelines.
  Package name match the upstream tarball name kigo-4.10.0.tar.xz
+ spec file name kigo.spec matches base package name
+ complies with all the legal guidelines:
  + License: (GPLv2 or GPLv3) and GFDL, matches actual license
  + No known patent problems
  + No emulator, no firmware, no binary-only or prebuilt components
+ COPYING (GNU GENERAL PUBLIC LICENSE Version 2), COPYING.DOC (GNU Free Documentation License Version 1.2) packaged as %doc
+ source matches upstream:
  MD5: 1b705678bc7f5ae883e23fa42563767d  kigo-4.10.0.tar.xz
  SHA1: 0dcee4587b9c57847bf8c2c7e0d2450c386aa969  kigo-4.10.0.tar.xz
  SHA256: c7436c465bd987227dd5b2cbf1102ed6b86cbdaae61d4c9ec576ea2f2bc1183e  kigo-4.10.0.tar.xz
+ builds on at least one arch
  build from mock is in F18 kde-unstable repo
+ no known non-working arches, so no ExcludeArch needed
+ no missing BuildRequires (builds in mock)
+ locales are handled properly by using %find_lang %{name} --with-kde macro
+ ldconfig call not needed (no shared libraries)
+ no duplicated system libraries
+ package not relocatable (no Prefix tag)
+ directory ownership correct (doesn't own directories owned by another package, owns all package-specific directories %{_kde4_appsdir}/%{name}/)
+ no duplicate files in %files
+ permissions correct, %defattr(-,root,root,-) not needed now, executables have executable permissions
+ macros used where possible (%{name}, %{version}, %{buildroot}, %{_target_platform}, %{cmake_kde4}, %{_kde4_datadir}, %{_kde4_bindir}, %{_kde4_iconsdir},%{_kde4_appsdir},%{_kde4_configdir})
+ non-code content: only permitted content, gamedata, themes under license that matches the code
+ no large documentation files, so no -doc package needed
+ no %doc files required at runtime
+ no header files, no -devel package needed
+ no static libraries, so no -static package needed
+ no devel symlinks which would need to be in a -devel subpackage
+ devel packages must require the base package (no -devel package)
+ no .la files
+ kigo.desktop file for the GUI app kigo present
+ desktop-file-validate is used in %check and the kigo.desktop file passes validation
+ all filenames are valid UTF-8
+ other packaging guidelines:
  + complies with the Filesystem Hierarchy Standard (all files in  %{_kde4_datadir}, %{_kde4_bindir}, %{_kde4_iconsdir},%{_kde4_appsdir},%{_kde4_configdir})
  + proper changelog, tags, BuildRequires, Summary, Description (got from kigo's documentation)
  + no non-UTF-8 characters
  + all relevant documentation included as %doc (COPYING, COPYING.DOC, README, AUTHORS)
  + RPM_OPT_FLAGS are used in %{cmake_kde4} macro
  + debuginfo package is valid (contains stripped symbols from ELF binary and source code related to it)
  + no rpaths (no check-rpaths error)
  + no configuration files, so %config guideline doesn't apply
  + no init scripts, so init script guideline doesn't apply
  + timestamps are preserved
  + %{?_smp_mflags} used
  + not a web application, so web application guideline doesn't apply
  + no conflicts (kdegames-4.10.0 is metapackage now which not includes apps)

+ license already included upstream
+ no translations for description and summary provided by upstream
+ package builds in mock (built for kde-unstable)
- successfully tested the package functionality (no testing yet)
+ scriptlets are sane (updating hicolor icon chache in %post, %postun, %posttrans)
+ subpackages other than devel should require the base package using a fully versioned dependency (no subpackages)
+ no .pc files, so "placement of .pc files" is irrelevant
+ no file dependencies
- package should contain man pages for binaries/scripts

Comment 7 nucleo 2013-02-19 16:50:59 EST
Edit summary in Review Request.
Comment 8 Rex Dieter 2013-02-19 17:10:58 EST
New Package SCM Request
Package Name: kigo
Short Description: Go Board game
Owners: than rdieter jreznik kkofler ltinkl rnovacek
Branches: f17 f18
Comment 9 Dennis Gilmore 2013-02-20 08:13:17 EST
Git done (by process-git-requests).
Comment 10 Rex Dieter 2013-02-21 16:34:25 EST