[Bug 520550] Review Request: IVAN - SDL roguelike
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=520550 Jason Tibbitts changed: What|Removed |Added Blocks|177841(FE-NEEDSPONSOR) | -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 520550] Review Request: IVAN - SDL roguelike
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=520550
--- Comment #13 from Michael Schwendt 2009-12-26
05:44:43 EDT ---
Re: comment 8
> ../../FeLib/Include/save.h:39: error: cast from 'FILE*' to 'truth' loses
> precision
| typedef.h:typedef int truth;
| typedef.h:typedef const int ctruth;
|
| save.h: FILE* Buffer;
| save.h: truth IsOpen() { return truth(Buffer); }
An attempt at creating a boolean type and really just ending with a typecast
from pointer to int. Nothing is won by not using an explicit NULL-pointer
comparison in this case.
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 520550] Review Request: IVAN - SDL roguelike
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=520550 --- Comment #12 from Bruno Wolff III 2009-10-11 20:04:21 EDT --- Since upstream isn't changing much, I don't think there will be too much difficulty keeping up with them. The main worry would be if some key library in Fedora now got obsoleted and you needed to make IVAN use a new library. That's likely far enough out that it is still worth packaging it now and dealing with it later if it happens. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 520550] Review Request: IVAN - SDL roguelike
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=520550 Ryan Rix changed: What|Removed |Added Flag|needinfo?(phrkonale...@gmai | |l.com) | --- Comment #11 from Ryan Rix 2009-10-11 19:20:44 EDT --- Unfortunately, getting this application to build on x86_64 is being more trouble than I thought. In the process, i've discovered a few other bugs. Without an upstream's assistance, I begin to wonder on whether this package will be maintainable long term to the quality the Fedora Project's packages should be... :( -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 520550] Review Request: IVAN - SDL roguelike
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=520550 Mamoru Tasaka changed: What|Removed |Added Flag||needinfo?(phrkonale...@gmai ||l.com) --- Comment #10 from Mamoru Tasaka 2009-10-11 11:35:00 EDT --- What is the status of this bug? -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 520550] Review Request: IVAN - SDL roguelike
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=520550 Jason Tibbitts changed: What|Removed |Added Status Whiteboard||NotReady BuildFails --- Comment #9 from Jason Tibbitts 2009-09-23 13:22:19 EDT --- Please clear the whiteboard when you have a package which builds properly in mock or koji. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 520550] Review Request: IVAN - SDL roguelike
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=520550 Jason Tibbitts changed: What|Removed |Added CC||ti...@math.uh.edu --- Comment #8 from Jason Tibbitts 2009-09-23 02:36:36 EDT --- This fails to build for me (x86_64, rawhide). There are many errors and my builder has lots of CPUs so the output is a bit tough to follow. Here's a scratch build; the builders have fewer CPUs so it's a bit easier to follow: http://koji.fedoraproject.org/koji/taskinfo?taskID=1700038 The errors all seem to be of the form: ../../FeLib/Include/save.h:39: error: cast from 'FILE*' to 'truth' loses precision Of course, you can do your own scratch builds, and it's a good idea to do them and include the URLs in your review tickets (to show that your package actually builds). Instructions are in http://fedoraproject.org/wiki/Join_the_package_collection_maintainers -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 520550] Review Request: IVAN - SDL roguelike
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=520550 --- Comment #7 from Ryan Rix 2009-09-15 23:11:09 EDT --- http://rrix.fedorapeople.org/ivan-0.50-1.fc11.src.rpm http://rrix.fedorapeople.org/ivan.spec Latest build, incorporating both Krzesimir and Michael's suggestions. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 520550] Review Request: IVAN - SDL roguelike
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=520550
--- Comment #6 from Ryan Rix 2009-09-15 20:35:41 EDT
---
[[> Patch1: %{name}-%{version}-fedora.patch
Dubious. The C++ fixes and the Makefile patch could be split into two patches.
So when a compiler update required further fixes, you could simply
rediff/enhance the C++ specific patch. But why is a patch to config.log and
config.status included? Those are files created at build-time.
]]
I'm not entirely sure... I suppose I need to dist-clean before I generate the
patch, correct?
I've implemented both of your suggestions (thank you both very much!) and will
rebuild the rpm after I get the patch all ironed out.
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 520550] Review Request: IVAN - SDL roguelike
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=520550
David Lawrence changed:
What|Removed |Added
Flag|needinfo- |
Michael Schwendt changed:
What|Removed |Added
CC||mschwe...@gmail.com
--- Comment #5 from Michael Schwendt 2009-09-15 14:12:04
EDT ---
Good review so far.
> I'm not sure about `%{__mkdir} -p
> %{buildroot}%{_datadir}/icons/hicolor/32x32/apps/'.
That's a directory that belongs into package "hicolor-icon-theme". This game
only stores a single %{name}.png in that directory, used by the desktop menu
entry. Adding "Requires: hicolor-icon-theme" would be more accurate and would
avoid unowned directories. Alternatively, the single file could be moved to
/usr/share/pixmaps.
> Requires: SDL
See https://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires
> BuildRequires: SDL-devel >= 1.2.0.
SDL in Fedora is > 1.2.0 for a very long time.
> %configure --bindir=%{_bindir} --datadir=%{_datadir}
> --localstatedir=%{_localstatedir}/games
The first two arguments are the default. Compare with "rpm --eval %configure".
> %{__cp} %{SOURCE1} %{buildroot}%{_datadir}/applnk/Applications/%{name}.desktop
Why do you copy it there only to remove it again when copying it elsewhere with
desktop-file-install?
> MUST: The spec file must be written in American English.
> Ok.
It ought to be "Rogue-like" not "roguelike", though.
> %changelog
> - initiql package
s/initiql/initial/ ;)
> File listed twice: /var/games/ivan/ivan-highscore.scores'
This is because the directory is included recursively and the highscores files
is also included explicitly. The directory entry could be marked %dir.
> Patch1: %{name}-%{version}-fedora.patch
Dubious. The C++ fixes and the Makefile patch could be split into two patches.
So when a compiler update required further fixes, you could simply
rediff/enhance the C++ specific patch. But why is a patch to config.log and
config.status included? Those are files created at build-time.
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 520550] Review Request: IVAN - SDL roguelike
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=520550
Krzesimir Nowak changed:
What|Removed |Added
CC||qdl...@gmail.com
--- Comment #4 from Krzesimir Nowak 2009-09-10 06:17:52 EDT
---
This is a prereview, so it is not formal. And maybe in some places I'm not
right - I'm trying to become a packager by doing informal prereviews.
MUST: rpmlint must be run on every package. The output should be posted in the
review.
Ok.
MUST: The package must be named according to the Package Naming Guidelines .
Ok.
MUST: The spec file name must match the base package %{name}, in the format
%{name}.spec unless your package has an exemption.
Ok.
MUST: The package must meet the Packaging Guidelines.
Fail - Game is named `Iter Vehemens ad Necem', but spec file says `Iter
Vehemens ad Necum'. Same error in .desktop file. It would be good to have patch
commented. `Patch0' rather should be used, instead of `Patch1' and `%patch0'
instead of `%patch1' in `%prep'.
MUST: The package must be licensed with a Fedora approved license and meet the
Licensing Guidelines .
Ok.
MUST: The License field in the package spec file must match the actual license.
Fail - license section in source code directs to LICENSE file, which states
that source is released under GPLv2+, not GPL+.
MUST: If (and only if) the source package includes the text of the license(s)
in its own file, then that file, containing the text of the license(s) for the
package must be included in %doc.
Fail/Ok - COPYING is included, but LICENSING file is not and this file is
mentioned in source code.
MUST: The spec file must be written in American English.
Ok.
MUST: The spec file for the package MUST be legible.
Ok.
MUST: The sources used to build the package must match the upstream source, as
provided in the spec URL. Reviewers should use md5sum for this task. If no
upstream URL can be specified for this package, please see the Source URL
Guidelines for how to deal with this.
Ok.
MUST: The package MUST successfully compile and build into binary rpms on at
least one primary architecture.
Ok - builds on i586.
MUST: If the package does not successfully compile, build or work on an
architecture, then those architectures should be listed in the spec in
ExcludeArch. Each architecture listed in ExcludeArch MUST have a bug filed in
bugzilla, describing the reason that the package does not compile/build/work on
that architecture. The bug number MUST be placed in a comment, next to the
corresponding ExcludeArch line.
??? - I didn't tested other architectures.
MUST: All build dependencies must be listed in BuildRequires, except for any
that are listed in the exceptions section of the Packaging Guidelines ;
inclusion of those as BuildRequires is optional. Apply common sense.
Ok.
MUST: The spec file MUST handle locales properly. This is done by using the
%find_lang macro. Using %{_datadir}/locale/* is strictly forbidden.
N/A.
MUST: Every binary RPM package (or subpackage) which stores shared library
files (not just symlinks) in any of the dynamic linker's default paths, must
call ldconfig in %post and %postun.
N/A.
MUST: If the package is designed to be relocatable, the packager must state
this fact in the request for review, along with the rationalization for
relocation of that specific package. Without this, use of Prefix: /usr is
considered a blocker.
N/A.
MUST: A package must own all directories that it creates. If it does not create
a directory that it uses, then it should require a package which does create
that directory.
Ok.
MUST: A Fedora package must not list a file more than once in the spec file's
%files listings.
Fail - during rpmbuild I got a warning:
`warning: File listed twice: /var/games/ivan/ivan-highscore.scores'
MUST: Permissions on files must be set properly. Executables should be set with
executable permissions, for example. Every %files section must include a
%defattr(...) line.
Well, this setgid is issue. But from what I can see, roguelikes are handled
that way.
MUST: Each package must have a %clean section, which contains rm -rf
%{buildroot} (or $RPM_BUILD_ROOT).
Ok.
MUST: Each package must consistently use macros.
Fail - there are `$RPM_BUILD_ROOT' and `%{buildroot}' macros mixed.
MUST: The package must contain code, or permissable content.
Ok.
MUST: Large documentation files must go in a -doc subpackage. (The definition
of large is left up to the packager's best judgement, but is not restricted to
size. Large can refer to either size or quantity).
Ok.
MUST: If a package includes something as %doc, it must not affect the runtime
of the application. To summarize: If it is in %doc, the program must run
properly if it is not present.
Ok.
MUST: Header files must be in a
[Bug 520550] Review Request: IVAN - SDL roguelike
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=520550 --- Comment #3 from Ryan Rix 2009-09-01 09:56:18 EDT --- http://rrix.fedorapeople.org/ <-- this contains SRPM and SPEC now; supersedes attached files, though at the moment they are the same. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 520550] Review Request: IVAN - SDL roguelike
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=520550 Ryan Rix changed: What|Removed |Added Version|rawhide |11 -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 520550] Review Request: IVAN - SDL roguelike
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=520550 --- Comment #1 from Ryan Rix 2009-09-01 00:30:54 EDT --- Created an attachment (id=359343) --> (https://bugzilla.redhat.com/attachment.cgi?id=359343) IVAN spec file -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 520550] Review Request: IVAN - SDL roguelike
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=520550 --- Comment #2 from Ryan Rix 2009-09-01 00:31:52 EDT --- Created an attachment (id=359344) --> (https://bugzilla.redhat.com/attachment.cgi?id=359344) IVAN srpm -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 520550] Review Request: IVAN - SDL roguelike
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=520550 Ryan Rix changed: What|Removed |Added Blocks||177841(FE-NEEDSPONSOR) Flag||needinfo- -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
