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 <dkl@xxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Flag|needinfo- | Michael Schwendt <mschwendt@xxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mschwendt@xxxxxxxxx --- Comment #5 from Michael Schwendt <mschwendt@xxxxxxxxx> 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@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review