Spec Name or Url: http://www.auroralinux.org/people/spot/review/oneko.spec SRPM Name or Url: http://www.auroralinux.org/people/spot/review/oneko-1.2-1.src.rpm Description: A cat (neko) chases the cursor (now a mouse) around the screen while you work. Alternatively, a dog chases a bone. Note to reviewers: This package is for FC-5. If you want to build for FC-4, you'd need to replace all the BuildRequires with: xorg-x11-devel.
To build on FC-4 you must also change the binary location in %files: %{_exec_prefix}/X11R6/bin/oneko I'll do a formal review later today once my devel box is available.
MUST items: - rpmlint output is clean - name is appropriate - spec filename matches package name - macro use in spec file is consistent (other than /usr/include; see below) - Build root is correct - Summary and description are ok. - No static libs - Desktop file looks ok and is installed correctly. - RPM_OPT_FLAGS passed to make. - Not relocatable - Contains code, not content - Package does not create any directories that it should own. - spec file is in english and is legible - Compiles and builds on devel (did not test in mock) - No shared libraries - No duplicate %files - Permissions set correctly - No -devel package needed SHOULD items: - No license file included. Please query upstream to include one, even if it just says that the software is public domain. - Builds on x86_64-FC4 (with noted changes) and i386-FC5 - Packages runs on both architectures Non-blocking issues: %{?_smp_mflags} not passed to make, but there is only one file to compile so it would have no effect. The tarball includes a japanese manpage. Why not include that in the package? The application itself has some small problems that should be reported upstream: 1) There is no way to turn off oneko. Once it's started, it runs until you log out of X or /usr/bin/kill it manually. I suspect that this was the purpose of the (removed) BSD daemon. 2) If you /usr/bin/kill oneko, the root window cursor reverts to an 'X', not a diagonal-arrow as is the default for gnome. Source does not match upstream due to the removal of the copyrighted images. All other files were verified the same with 'diff'. MUSTFIX: * Several of the included documentation files are in Japanese. This should probably be indicated in the filename somehow. * The 'make' line in the specfile uses "-I /usr/include". Please change this to"-I %{_includedir}". * The patch to the manpage has a grammatical problem: "a cat wite tiger-like stripe" should be "a cat with tiger-like stripes" The manpage has other grammatical problems that I'm willing to ignore, but this line was explicitly fixed by the patch, so it should at least be grammatically correct. :) * BR: xorg-x11-proto-devel is redundant since it's required by libX11-devel. * 'install' is missing '-p' Questions: The spec file says the license is "Public Domain", but I could not find reference to any license information in the tarball or on the website. Is this the proper license to use if the author hasn't specified one? Have you asked upstream what kind of license it falls under?
The upstream maintainers have vanished into the ether... see the licensing discussion here: http://www.monkey.org/openbsd/archive/ports/0010/msg00009.html I tend to go with what Debian decided, which is public domain. - Added the Japanese manpage. - The cursor change seems temporary, as X seems to eventually put the cursor back to normal... someone who understands X's internals better than me is welcome to try and fix oneko. - The removed "BSD Daemon" was a set of graphics for oneko that turned it from a cat into the BSD logo, has nothing to do with its non existent shutdown. - Doc files renamed with ".jp" extension where appropriate. - Used %{_includedir} - Removed _i386_ define (as it will not be always true) - fix typo - remove redundant BR - add -p to install New package here: SRPM: http://www.auroralinux.org/people/spot/review/oneko-1.2-2.src.rpm SPEC: http://www.auroralinux.org/people/spot/review/oneko.spec
I'm comfortable with the license as PD given this licensing discussion. Only one comment: The japanese man page does not need the '.jp' extension since it already lives in a 'ja' subdirectory. Everything else looks good as per comment #2. Fix the japanese manpage name and I'll approve it.
Small side-note here: > The 'make' line in the specfile uses "-I /usr/include". > Please change this to"-I %{_includedir}". Preferably, neither one. /usr/include is in default search path list. By adding -I %{_includedir}, you defeat the purpose of a user defined search path list which takes precedence over the default search path list.
(In reply to comment #5) > Small side-note here: > > > The 'make' line in the specfile uses "-I /usr/include". > > Please change this to"-I %{_includedir}". > > Preferably, neither one. /usr/include is in default search path list. > By adding -I %{_includedir}, you defeat the purpose of a user defined > search path list which takes precedence over the default search path list. I verified that "-I %{_includedir}" isn't needed to build this package. It should be removed.
Ping spot.
-3 removes the includedir from CFLAGS, and renames the japanese man page: SRPM: http://www.auroralinux.org/people/spot/review/oneko-1.2-3.src.rpm SPEC: http://www.auroralinux.org/people/spot/review/oneko.spec
All issues addressed. APPROVED