Bug 746520

Summary: Review Request: findthatword - A word search maker
Product: [Fedora] Fedora Reporter: Gwyn Ciesla <gwync>
Component: Package ReviewAssignee: Jerry James <loganjerry>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: rawhideCC: loganjerry, notting, package-review
Target Milestone: ---Flags: loganjerry: fedora-review+
gwync: fedora-cvs+
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: findthatword-0.1-3.fc16 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2011-11-18 18:39:01 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Gwyn Ciesla 2011-10-16 20:10:32 UTC
A word search maker for teachers, parents and those who enjoy creating
puzzles. FindThatWord aims to be the best free wordsearch creation
program available with useful features such as clues, hidden messages
and automated page layout.

SRPM: http://zanoni.jcomserv.net/fedora/findthatword/findthatword-0.1-1.fc15.src.rpm
SPEC: http://zanoni.jcomserv.net/fedora/findthatword/findthatword.spec

Comment 1 Jerry James 2011-10-17 22:21:42 UTC
I'll take this review.  Consider it payback for your review of undertaker. :-)

Comment 2 Jerry James 2011-10-17 22:51:28 UTC
There are some unnecessary elements of this spec file, unless you intend to also use it for EPEL.  They are:
- The python_sitelib definition at the top (now defined in /usr/lib/rpm/macros)
- the BuildRoot tag
- the "-p0" on the %patch invocations
- "rm -rf $RPM_BUILD_ROOT" at the start of %install
- the %clean script
- %defattr in the %files section

Since the icon is an SVG, it should be installed into %{_datadir}/icons/hicolor/scalable/apps instead of its current location.

This package creates %{python_sitelib}/findthatword, then doesn't put the Python files into it; they're directly inside %{python_sitelib} instead.

The findthatword.desktop file uses a deprecated tag, Encoding: http://standards.freedesktop.org/desktop-entry-spec/latest/apc.html.

+: OK
-: must be fixed
=: should be fixed (at your discretion)
N: not applicable

MUST:
[+] rpmlint output:
findthatword.noarch: W: spelling-error %description -l en_US wordsearch -> word search, word-search, Wordsworth
findthatword.noarch: W: no-manual-page-for-binary findthatword
1 packages and 1 specfiles checked; 0 errors, 2 warnings.

The description has one instance of "word search" and one of "wordsearch", so you probably should change the latter to match the former.

[+] follows package naming guidelines
[+] spec file base name matches package name
[+] package meets the packaging guidelines
[+] package uses a Fedora approved license
[+] license field matches the actual license
[+] license file is included in %doc
[+] spec file is in American English
[+] spec file is legible
[+] sources match upstream: md5sum is 025886ae0f30dc1cabeebca21987c82f for both
[+] package builds on at least one primary arch (tried x86_64)
[N] appropriate use of ExcludeArch
[+] all build requirements in BuildRequires
[N] spec file handles locales properly
[N] ldconfig in %post and %postun
[+] no bundled copies of system libraries
[N] no relocatable packages
[+] package owns all directories that it creates
[+] no files listed twice in %files
[+] proper permissions on files
[+] consistent use of macros
[+] code or permissible content
[N] large documentation in -doc
[+] no runtime dependencies in %doc
[N] header files in -devel
[N] static libraries in -static
[N] .so in -devel
[N] -devel requires main package
[+] package contains no libtool archives
[+] package contains a desktop file, uses desktop-file-install
[+] package does not own files/dirs owned by other packages
[+] all filenames in UTF-8

SHOULD:
[N] query upstream for license text
[N] description and summary contains available translations
[+] package builds in mock: tried fedora-rawhide-i386
[+] package builds on all supported arches: tried i386 and x86_64
[+] package functions as described: minimal testing only
[+] sane scriptlets
[N] subpackages require the main package
[N] placement of pkgconfig files
[N] file dependencies versus package dependencies
[+] package contains man pages for binaries/scripts: yes, technically there should be a man page for findthatword(1), but since it would be trivial, I don't see the point.

Comment 3 Gwyn Ciesla 2011-10-18 00:26:18 UTC
Thanks. Potential t-shirt:  Fedora:  Where no good deed goes unpunished. :)

Icon placement, .desktop key and sitelib subdir fixed.  Leaving the rest because I think I may wind up building this for EPEL.

SRPM:
http://zanoni.jcomserv.net/fedora/findthatword/findthatword-0.1-2.fc15.src.rpm
SPEC: http://zanoni.jcomserv.net/fedora/findthatword/findthatword.spec

Comment 4 Jerry James 2011-10-18 03:42:32 UTC
Looks good.  I still think you should do s/wordsearch/word search/ on the description, but I'll leave that to your discretion.  APPROVED.

Oh, and I'll buy one of those t-shirts. :-)

Comment 5 Gwyn Ciesla 2011-10-18 12:14:33 UTC
Awesome, thanks for the review!

New Package SCM Request
=======================
Package Name: findthatword
Short Description: A word search maker
Owners: limb
Branches: f16 f15 EL-6 EL-5
InitialCC:

Comment 6 Gwyn Ciesla 2011-10-18 12:15:36 UTC
Git done (by process-git-requests).

Comment 7 Fedora Update System 2011-10-18 13:48:53 UTC
findthatword-0.1-2.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/findthatword-0.1-2.fc15

Comment 8 Fedora Update System 2011-10-18 13:49:01 UTC
findthatword-0.1-2.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/findthatword-0.1-2.el6

Comment 9 Fedora Update System 2011-10-18 13:49:09 UTC
findthatword-0.1-2.el5 has been submitted as an update for Fedora EPEL 5.
https://admin.fedoraproject.org/updates/findthatword-0.1-2.el5

Comment 10 Fedora Update System 2011-10-18 13:49:16 UTC
findthatword-0.1-2.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/findthatword-0.1-2.fc16

Comment 11 Fedora Update System 2011-10-18 19:32:38 UTC
findthatword-0.1-2.el6 has been pushed to the Fedora EPEL 6 testing repository.

Comment 12 Fedora Update System 2011-11-18 18:39:01 UTC
findthatword-0.1-3.el6 has been pushed to the Fedora EPEL 6 stable repository.

Comment 13 Fedora Update System 2011-11-18 18:40:05 UTC
findthatword-0.1-3.el5 has been pushed to the Fedora EPEL 5 stable repository.

Comment 14 Fedora Update System 2011-11-19 05:56:58 UTC
findthatword-0.1-3.fc15 has been pushed to the Fedora 15 stable repository.

Comment 15 Fedora Update System 2011-11-19 06:04:56 UTC
findthatword-0.1-3.fc16 has been pushed to the Fedora 16 stable repository.