Hi!

I don't have rights to make a review but I may help you cleaning up the spec :]

- first of all: read the output from the build process:

 scrollkeeper-update -p /var/scrollkeeper -o
/var/tmp/quarry-0.1.15-1-root-rpm-build/usr/share/omf/quarry
Could not create directory /var/scrollkeeper : Permission denied
Cannot write to log file: /var/log/scrollkeeper.log : Permission denied
Could not create database.  Aborting update.
Cannot write to log file: /var/log/scrollkeeper.log : Permission denied
make[4]: [install-data-hook-omf] Error 1 (ignored)

I have prepared a small patch to resolve this problem. You will also need to add
this stuff:
http://fedoraproject.org/wiki/ScriptletSnippets#head-3c9f517f0cd4aaabb369a8805226d85dc2f02793

warning: File listed twice: /usr/share/omf/quarry/quarry-C.omf
warning: File listed twice: /usr/share/quarry/gtkrc
warning: File listed twice: /usr/share/quarry/help
warning: File listed twice: /usr/share/quarry/help/C
warning: File listed twice: /usr/share/quarry/help/C/fdl.xml
warning: File listed twice: /usr/share/quarry/help/C/figures
warning: File listed twice: /usr/share/quarry/help/C/figures/control-center.png
warning: File listed twice: /usr/share/quarry/help/C/legal.xml
warning: File listed twice: /usr/share/quarry/help/C/quarry.html
warning: File listed twice: /usr/share/quarry/help/C/quarry.xml
warning: File listed twice: /usr/share/quarry/help/quarry-help.css
warning: File listed twice: /usr/share/quarry/markup-themes
warning: File listed twice: /usr/share/quarry/markup-themes/bold
warning: File listed twice: /usr/share/quarry/markup-themes/bold/circle.svg
warning: File listed twice: /usr/share/quarry/markup-themes/bold/cross.svg
warning: File listed twice: /usr/share/quarry/markup-themes/bold/last-move.svg
warning: File listed twice: /usr/share/quarry/markup-themes/bold/selected.svg
warning: File listed twice: /usr/share/quarry/markup-themes/bold/square.svg
warning: File listed twice: /usr/share/quarry/markup-themes/bold/theme.cfg
warning: File listed twice: /usr/share/quarry/markup-themes/bold/triangle.svg
warning: File listed twice: /usr/share/quarry/markup-themes/default
warning: File listed twice: /usr/share/quarry/markup-themes/default/circle.svg
warning: File listed twice: /usr/share/quarry/markup-themes/default/cross.svg
warning: File listed twice: /usr/share/quarry/markup-themes/default/last-move.svg
warning: File listed twice: /usr/share/quarry/markup-themes/default/selected.svg
warning: File listed twice: /usr/share/quarry/markup-themes/default/square.svg
warning: File listed twice: /usr/share/quarry/markup-themes/default/theme.cfg
warning: File listed twice: /usr/share/quarry/markup-themes/default/triangle.svg
warning: File listed twice: /usr/share/quarry/markup-themes/filled
warning: File listed twice: /usr/share/quarry/markup-themes/filled/circle.svg
warning: File listed twice: /usr/share/quarry/markup-themes/filled/cross.svg
warning: File listed twice: /usr/share/quarry/markup-themes/filled/last-move.svg
warning: File listed twice: /usr/share/quarry/markup-themes/filled/selected.svg
warning: File listed twice: /usr/share/quarry/markup-themes/filled/square.svg
warning: File listed twice: /usr/share/quarry/markup-themes/filled/theme.cfg
warning: File listed twice: /usr/share/quarry/markup-themes/filled/triangle.svg
warning: File listed twice: /usr/share/quarry/textures
warning: File listed twice: /usr/share/quarry/textures/wood1.jpg
warning: File listed twice: /usr/share/quarry/textures/wood2.jpg

You should wrote:

%{_datadir}/omf/quarry/
%{_datadir}/quarry/

instead of:

%{_datadir}/omf/quarry
%{_datadir}/omf/quarry/quarry*.omf
%{_datadir}/quarry
%{_datadir}/quarry/*

Michael Schwendt gave a good explanation of this problem here â
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=165616#c8

- use variables in "Source0" tag. You will have less work with new releases:

Source0:        http://download.gna.org/quarry/%{name}-%{version}.tar.gz

- use %find_lang macro
- please provide menu entry:
http://fedoraproject.org/wiki/Extras/FedoraDesktopEntryGuidelines
- you don't have to add dist tag in the changelog

Everything else looks good to me :]

Created attachment 117825 [details]
Disables running scrollkeeper at the build time.

Created attachment 117827 [details]
quarry.desktop file

Please change "Comment" field ;)

Created attachment 117828 [details]
patch to the spec file

(In reply to comment #1)
> - use variables in "Source0" tag. You will have less work with new releases:
> 
> Source0:        http://download.gna.org/quarry/%{name}-%{version}.tar.gz

I would avoid using the %{name} macro in Source: lines (and in fact most other
places); whilst %{version} may change with each release, %{name} should hardly
ever change and the spec file will be clearer if the package name is used
instead. The value of the %{name} macro comes when templates are used for spec
files; a hand-crafted spec file will generally be better off not using it.

Thanks, Dawid and Paul. Dawid, I actually added a desktop file already - but
didn't bump up the revision number, since I did it within an hour and didn't
think anyone would have downloaded the originals. Sorry for that!

http://hircus.org/fedora/quarry/quarry.spec
http://hircus.org/fedora/quarry/quarry-0.1.15-1.fc4.src.rpm

Small trademark nitpick, Othello I believe is a trademark and you should
probably make note of that in the description text.

Othello is a registered trademark of Tsukuda Original, licensed by Anjar Co.,
copyright 1973, 1990 Pressman Toy Corporation

For reference see this thread in fedora-extras-list about appropriate ways to
deal with trademarks that show up in the descriptive text like this.

https://www.redhat.com/archives/fedora-extras-list/2005-July/msg00161.html

-jef

http://hircus.org/fedora/quarry/quarry.spec
http://hircus.org/fedora/quarry/quarry-0.1.15-3.fc4.src.rpm (*)

Revision 3, with trademark notice added. Thanks Jef! So that's why the game's
also called Reversi (or Iagno in case of gnome-games..)

(*) My previous comment should have said 2.fc4 not 1.fc4, oops.

Few remarks:
- you still don't use find_lang macro :P

Remove this line:

%{_datadir}/locale/*/LC_MESSAGES/quarry.mo

and change "%files" to:

%files -f %{name}.lang

You have to do this in this way because "%_install_langs" macro in
/root/.rpmmacros file will not work. find_lang macro allows users to install
only _needed_ translations.
- in my build envinronment this line is unnecessary:

mkdir -p $RPM_BUILD_ROOT%{_datadir}/applications

desktop-file-install creates this directory automatically
- close brackets at the end of your e-mail adress in the changelog section
- please add to the desktop file "GenericName" tag, for example:

GenericName=Multi-purpose board game GUI

- IMHO line "X-Desktop-File-Install-Version=0.10" in the desktop file is
unnecessary. desktop-file-install program will add it automatically.

Ah yes. I called %{find_lang} but did not make use of it in %files before. Thanks!

Updated the scrollkeeper patch - after your patch it was still trying to create
/var/scrollkeeper, the ${mkinstalldirs} was still there.

Added GenericName and modified Comment - seems that GenericName should be
shorter (Board Game GUI) and Comment longer (Multi-purpose board game GUI ...)

Many thanks Dawid.

http://hircus.org/fedora/quarry/quarry-0.1.15-4.fc4.src.rpm

The first scrollkeeper patch only applied if the Makefile.in's are regenerated,
and they need automake 1.7 - currently resorting to patching help/C/Makefile.in
manually, will contact upstream about this.

Dawid, want to be a reviewer for the package? Turns out the pre-Extras packaging
I did for fedora.us makes me eligible for CVS access already, so technically
this is not a first submission anymore.

Thanks,

- Michel

(In reply to comment #12)
> Dawid, want to be a reviewer for the package?

Yes, I would like to. I requested today membership in the fedorabugs group but
I'm not shure when I'll get the reply.

I would appreciate it if someone could review this package in the meantime. I
don't want to be a bottleneck :D

> so technically this is not a first submission anymore.

So you have a sponsor already?

(In reply to comment #13)
> Yes, I would like to. I requested today membership in the fedorabugs group but
> I'm not shure when I'll get the reply.
> 

Dawid, you should now be able to reassign review bugs.

Dave

> Dawid, you should now be able to reassign review bugs.

Thanks!

Here's my review:
* no rpmlint warnings
* license is GPL
* source matches upstream
* uses find_lang macro ;-)
* scriptlets look OK

Approved :D

With regard to the trademark, where is the sense in mentioning Othello
when apparently you could never use that trademark for an implementation
of the game based on this GUI framework without licencing the name Othello?
E.g. see  http://www.atmedia.net/TWWWEP/noothello.html

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

http://hircus.org/fedora/quarry/quarry-0.1.15-5.fc4.src.rpm
http://hircus.org/fedora/quarry/quarry-cleanup.sh
http://hircus.org/fedora/quarry/quarry.spec

All occurences of Othello replaced with Reversi, and relevant files renamed.
I have to package my own tarball to be totally clean, to check that the
sources are otherwise pristine, please download the original tarball from
the commented-out source line in the spec file, and then apply the cleanup
script linked above.

- - Michel
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2 (GNU/Linux)

iD8DBQFDG4toWt0J3fd+7ZARAoUEAKCC4EGTLDgn5uyA2zJOcLoKXsKHqgCbBnEy
q4lbn09tk67kZnEYtK8LlU8=
=Xpy2
-----END PGP SIGNATURE-----

The rest of this package was approved by Dawid in comment 15 already. And I
believe there's no need to compare/verify the modified source tarball. Just
don't forget the re-packaging for updates. ;-)

Hmmm... how about changing tarball name to something like
quarry-0.1.15.patched.tar.bz2 or quarry-0.1.15-notrademark.tar.bz2 to inform
users that the sources were modified? Please take a look at XMMS or OpenSSH
packages:
http://cvs.fedora.redhat.com/viewcvs/*checkout*/rpms/xmms/devel/sources?root=extras
http://cvs.fedora.redhat.com/viewcvs/*checkout*/rpms/openssh/devel/sources

Everything else looks good so feel free to import quarry to CVS :)

Ping ;-)

BTW there's a new release of Quarry â
https://mail.gna.org/public/quarry-announce/2005-10/msg00000.html

Still alive! Been a bit busy, and after discussion with Paul I decided to modify
both my quarry and grhino patches to only change the
game-name-that-shan't-be-mentioned to Reversi within the UI code.

Hopefully I'll get it up this weekend! Will probably need retesting though.

Done!