Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 166087 - Review Request: quarry - A multi-purpose board game GUI
Summary: Review Request: quarry - A multi-purpose board game GUI
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Dawid Gajownik
QA Contact: David Lawrence
URL: http://hircus.org/fedora/quarry/
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2005-08-16 20:10 UTC by Michel Alexandre Salim
Modified: 2007-11-30 22:11 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2005-11-17 05:10:31 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
Disables running scrollkeeper at the build time. (deleted)
2005-08-17 09:07 UTC, Dawid Gajownik
no flags Details | Diff
quarry.desktop file (deleted)
2005-08-17 09:09 UTC, Dawid Gajownik
no flags Details
patch to the spec file (deleted)
2005-08-17 09:10 UTC, Dawid Gajownik
no flags Details | Diff

Description Michel Alexandre Salim 2005-08-16 20:10:43 UTC
Spec Name or Url: http://hircus.org/fedora/quarry/quarry.spec
SRPM Name or Url: http://hircus.org/fedora/quarry/quarry-0.1.15-1.fc4.src.rpm
Description: 
Quarry is a multi-purpose GUI for several board games, at present Go, Amazons
and Othello. It allows users to play against computer players (third-party
programs, e.g. GNU Go or GRhino) or other humans, view and edit game records.
Future versions will also support Internet game servers and provide certain
features for developers of board game-playing engines for enhancing their
programs.

Comment 1 Dawid Gajownik 2005-08-17 09:05:55 UTC
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 :]

Comment 2 Dawid Gajownik 2005-08-17 09:07:23 UTC
Created attachment 117825 [details]
Disables running scrollkeeper at the build time.

Comment 3 Dawid Gajownik 2005-08-17 09:09:13 UTC
Created attachment 117827 [details]
quarry.desktop file

Please change "Comment" field ;)

Comment 4 Dawid Gajownik 2005-08-17 09:10:14 UTC
Created attachment 117828 [details]
patch to the spec file

Comment 5 Paul Howarth 2005-08-17 09:23:42 UTC
(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.



Comment 6 Michel Alexandre Salim 2005-08-18 17:34:22 UTC
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

Comment 7 Jef Spaleta 2005-08-18 21:38:57 UTC
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



Comment 8 Michel Alexandre Salim 2005-08-18 23:42:25 UTC
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.

Comment 9 Dawid Gajownik 2005-08-19 08:17:51 UTC
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.

Comment 10 Michel Alexandre Salim 2005-08-19 17:00:58 UTC
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.

Comment 11 Michel Alexandre Salim 2005-08-19 17:57:27 UTC
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.

Comment 12 Michel Alexandre Salim 2005-09-02 02:42:32 UTC
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

Comment 13 Dawid Gajownik 2005-09-02 18:12:29 UTC
(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?

Comment 14 David Lawrence 2005-09-02 18:48:34 UTC
(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

Comment 15 Dawid Gajownik 2005-09-02 19:26:24 UTC
> 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

Comment 16 Michael Schwendt 2005-09-03 11:34:10 UTC
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

Comment 17 Michel Alexandre Salim 2005-09-05 00:03:11 UTC
-----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-----


Comment 18 Michael Schwendt 2005-09-05 11:16:43 UTC
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. ;-)

Comment 19 Dawid Gajownik 2005-09-05 12:14:42 UTC
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 :)

Comment 20 Dawid Gajownik 2005-10-31 20:45:45 UTC
Ping ;-)

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

Comment 21 Michel Alexandre Salim 2005-11-03 23:26:43 UTC
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.

Comment 22 Michel Alexandre Salim 2005-11-17 05:10:31 UTC
Done!


Note You need to log in before you can comment on or make changes to this bug.