Bug 166087 - Review Request: quarry - A multi-purpose board game GUI
Review Request: quarry - A multi-purpose board game GUI
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Dawid Gajownik
David Lawrence
http://hircus.org/fedora/quarry/
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2005-08-16 16:10 EDT by Michel Alexandre Salim
Modified: 2007-11-30 17:11 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2005-11-17 00:10:31 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)
Disables running scrollkeeper at the build time. (813 bytes, patch)
2005-08-17 05:07 EDT, Dawid Gajownik
no flags Details | Diff
quarry.desktop file (221 bytes, text/plain)
2005-08-17 05:09 EDT, Dawid Gajownik
no flags Details
patch to the spec file (1.76 KB, patch)
2005-08-17 05:10 EDT, Dawid Gajownik
no flags Details | Diff

  None (edit)
Description Michel Alexandre Salim 2005-08-16 16:10:43 EDT
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 05:05:55 EDT
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 05:07:23 EDT
Created attachment 117825 [details]
Disables running scrollkeeper at the build time.
Comment 3 Dawid Gajownik 2005-08-17 05:09:13 EDT
Created attachment 117827 [details]
quarry.desktop file

Please change "Comment" field ;)
Comment 4 Dawid Gajownik 2005-08-17 05:10:14 EDT
Created attachment 117828 [details]
patch to the spec file
Comment 5 Paul Howarth 2005-08-17 05:23:42 EDT
(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 13:34:22 EDT
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 17:38:57 EDT
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 19:42:25 EDT
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 04:17:51 EDT
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 13:00:58 EDT
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 13:57:27 EDT
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-01 22:42:32 EDT
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 14:12:29 EDT
(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 14:48:34 EDT
(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 15:26:24 EDT
> 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 07:34:10 EDT
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-04 20:03:11 EDT
-----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 07:16:43 EDT
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 08:14:42 EDT
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 15:45:45 EST
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 18:26:43 EST
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 00:10:31 EST
Done!

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