[Bug 189713] Review Request: gnubg
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Review Request: gnubg https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=189713 --- Additional Comments From [EMAIL PROTECTED] 2006-06-06 05:31 EST --- * Tue Jun 6 2006 - Joost Soeterbroek [EMAIL PROTECTED] - 20060530-5 - minor change in man file conversion, move from install to prep Spec URL: http://www.soeterbroek.com/linux/fedora/extras/gnubg/gnubg.spec SRPM URL: http://www.soeterbroek.com/linux/fedora/extras/gnubg/gnubg-20060530-5.src.rpm -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 189713] Review Request: gnubg
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Review Request: gnubg https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=189713 --- Additional Comments From [EMAIL PROTECTED] 2006-06-06 05:34 EST --- (In reply to comment #14) * I see you've put the png ion the 32x32 dir is it (circa) 32x32 pixels? if not it should probably be in another dir (for example 48x48 or ... see the ones on your system). gnubg.png is is 32x32 pixels exacltly. -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 189713] Review Request: gnubg
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Review Request: gnubg https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=189713 [EMAIL PROTECTED] changed: What|Removed |Added OtherBugsDependingO|163778 |163779 nThis|| --- Additional Comments From [EMAIL PROTECTED] 2006-06-06 14:39 EST --- Looks good now, approved! -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 189713] Review Request: gnubg
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Review Request: gnubg https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=189713 --- Additional Comments From [EMAIL PROTECTED] 2006-06-05 15:35 EST --- * Sun Jun 5 2006 - Joost Soeterbroek [EMAIL PROTECTED] - 20060530-4 - added BuildReqs desktop-file-utils Spec URL: http://www.soeterbroek.com/linux/fedora/extras/gnubg/gnubg.spec SRPM URL: http://www.soeterbroek.com/linux/fedora/extras/gnubg/gnubg-20060530-4.src.rpm -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 189713] Review Request: gnubg
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Review Request: gnubg https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=189713 --- Additional Comments From [EMAIL PROTECTED] 2006-06-05 16:23 EST --- Hi, just a few quick remarks I take a closer look soon: * # convert man page from UTF8 to ISO-8859-1 Actually its the other way around :) * /usr/bin/iconv -f ISO-8859-1 -t UTF8 $RPM_BUILD_ROOT/usr/share/man/man6/gnubg.6 /tmp/foo /bin/mv /tmp/foo $RPM_BUILD_ROOT/usr/share/man/man6/gnubg.6 Don't use a file in the global temp dir for this and don't do this in %install do it in %prep and then use gnubg.6.tmp as tempfile name, what happens when using the global /tmp dir and another packages which gets build at the same time does the same? Also by using a fixed name in /tmp you're creating a tempfile security vulnerability (on the buildsys). * I see you've put the png ion the 32x32 dir is it (circa) 32x32 pixels? if not it should probably be in another dir (for example 48x48 or ... see the ones on your system). -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 189713] Review Request: gnubg
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.
Summary: Review Request: gnubg
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=189713
--- Additional Comments From [EMAIL PROTECTED] 2006-06-03 03:46 EST ---
MUST:
=
* rpmlint output is:
W: gnubg file-not-utf8 /usr/share/man/man6/gnubg.6.gz
W: gnubg-databases no-documentation
W: gnubg-sounds no-documentation
You must fix the not utf8 error, you can do so by running iconv
on the uncompressed manpage, see gcompris for an example. Notice that the
original encoding may be different then in gcompris though (probably not).
* Package and spec file named appropriately
* Packaged according to packaging guidelines
* License (GPL) ok, license file included
* spec file is legible and in Am. English.
* Source matches upstream
* Compiles and builds on devel-x86_64
* BR: see below
* No locales
* no shared libs
* Not relocatable
* Package owns / or requires all dirs
* No duplicate files Permissions ok
* %clean macro usage OK
* Contains code only
* %doc does not affect runtime, and isn't large enough to warrent a sub package
* no -devel needed
* gui .desktop required but none included! must fix!
MUST fix:
=
* building on devel gives:
error: Installed (but unpackaged) file(s) found:
/usr/share/info/dir
This file should be removed at the end of %install
Be sure to use rm -f for the removal since in my experience this file
doesn't get created on all Fedora versions.
* Remove bogus BuildRequires: gtk+-devel this is a gtk2 app, gtk+-devel
is for gtk1 apps.
* gtk2-devel implies freetype-devel, remove Extranous freetype-devel BR.
* esound-devel implies audiofile-devel, remove Extranous audiofile-devel BR.
* Add a desktop file and icon, see njam(.spec) for an example on howto
properly install the .desktop file and the icon and howto properly update the
icon cache on install / remove.
Should fix:
===
* I get this during the build:
/bin/sh: gnubg.weights: No such file or directory
make[2]: [gnubg.wd] Error 1 (ignored)
Yet you've included a a gnubg.weights as Source1, appearantly its not put
in the right place.
* And this:
cp: cannot stat /usr/share/texinfo/texinfo.dtd': No such file or directory
But this one can brobably be ignored I can't find texinfo.dtd anywhere on
my fairly complete install. Notice that gnubg itself does contain and
installs a texinfo.dtd
* You can write things like:
%dir %{_datadir}/gnubg/met
%{_datadir}/gnubg/met/*
As just:
%{_datadir}/gnubg/met
And the same for:
%dir %{_datadir}/gnubg/scripts
%{_datadir}/gnubg/scripts/*
Which can be just:
%{_datadir}/gnubg/scripts
Etc.
And probably the same for:
%dir %{_datadir}/gnubg/doc
%{_datadir}/gnubg/doc/*.png
%{_datadir}/gnubg/doc/texinfo.dtd
%{_datadir}/gnubg/doc/gnubg.xml
Which can probably be written as just:
%{_datadir}/gnubg/doc
Explain:
* Why add --without-board3d, gtkglext(-devel) is available in FE.
* Why the seperate database and soundpackages, does the main package
function without these?
--
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the QA contact for the bug, or are watching the QA contact.
___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 189713] Review Request: gnubg
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.
Summary: Review Request: gnubg
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=189713
--- Additional Comments From [EMAIL PROTECTED] 2006-06-03 03:52 EST ---
And a MUST fix I missed initially:
* You must %ghost the .pyo files in
/usr/share/gnubg/scripts/
You can do this by adding this line to %files:
%ghost %{_datadir}/gnubg/scripts/*.pyo
--
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the QA contact for the bug, or are watching the QA contact.
___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 189713] Review Request: gnubg
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Review Request: gnubg https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=189713 --- Additional Comments From [EMAIL PROTECTED] 2006-06-01 09:23 EST --- rpmlint on gnubg-20060530-1.src.rpm gives E: gnubg configure-without-libdir-spec -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 189713] Review Request: gnubg
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.
Summary: Review Request: gnubg
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=189713
[EMAIL PROTECTED] changed:
What|Removed |Added
Status|NEW |ASSIGNED
AssignedTo|[EMAIL PROTECTED] |[EMAIL PROTECTED]
OtherBugsDependingO|163776 |163778
nThis||
--- Additional Comments From [EMAIL PROTECTED] 2006-06-01 10:39 EST ---
No formal review, just the results of a quick scan, which has turned up enough
for you to work on for now:
Why call autogen.sh? thats concidered bad practice unless absolutly nescesarry,
and if you must you should do so from %setup not %install
Also BR: automake implies autoconf, so the BR: autoconf is extranous and should
be removed. (Notice if you remove the autogen.sh call that you then should also
remove the autoxxx and libtool BR's).
Also BR:L glib2 is plain wrong this is a gtk1 app right, so it should be
glib-devel, which is implied by BR: gtk+-devel, please remove the BR glib2.
Don't call ./configure yourself instead use %configure (that will fix the
rpmlint error) you can call it with your own args added like this:
%configure --with-python \
--without-gdbm \
--without-guile \
--without-timecontrol \
--without-board3d
make prefix=$RPM_BUILD_ROOT%{_prefix} install-strip
is dead wrong, use:
make install DESTDIR=$RPM_BUILD_ROOT
Stripping is wrong, rpmbuild will do this for you, and if you DIY, the
-debuginfo package will be useless.
Unowned directories:
%{_datadir}/gnubg
%{_datadir}/gnubg/met
%{_datadir}/gnubg/doc
%{_datadir}/gnubg/scripts
%{_datadir}/gnubg/sounds
--
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the QA contact for the bug, or are watching the QA contact.
___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 189713] Review Request: gnubg
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Review Request: gnubg https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=189713 --- Additional Comments From [EMAIL PROTECTED] 2006-06-01 13:55 EST --- (In reply to comment #6) (In reply to comment #5) Why call autogen.sh? thats concidered bad practice unless absolutly I am calling autogen.sh because the source is nightly tarball from CVS not containing a configure file. The latest available source I found is contained in a SRPM more than a year old (http://www.acepoint.de/GnuBG/redhat/gnubg-0.15-1.src.rpm). The nightly CVS tarballs seem to be the only recent published sources available. Yes, Thats a good reason, I should have spotted that. myself :) Anyways all the other points still remain to be fixed. -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 189713] Review Request: gnubg
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Review Request: gnubg https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=189713 [EMAIL PROTECTED] changed: What|Removed |Added CC||[EMAIL PROTECTED] --- Additional Comments From [EMAIL PROTECTED] 2006-05-29 02:28 EST --- Joost, I noticed you put this game on the Games SIG review list (good!). But thats not entirely how it works, we (the Games SIG) have got this sortof unwritten rule of a review for a review. So shall we exchange reviews? I currently need a reviewer for: https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=192060 Also, before I do a review I would like you to post a new version fixing the issues mentioned in comment 1 -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
