[Bug 440] Review request: vbam - Nintendo Gameboy, Gameboy Color, and Gameboy Advance Emulator
http://bugzilla.rpmfusion.org/show_bug.cgi?id=440
--- Comment #5 from Dave Straub <dave(a)is1337.net> 2009-03-16 20:08:23 ---
(In reply to comment #4)
Some comments:
* You should specify a version to checkout from upstream VCS. Otherwise the
reviewer cannot be sure to compare/recreate the same exact version you used.
Place this details above the Source tag.
https://fedoraproject.org/wiki/Packaging/SourceURL#Using_Revision_Control
The revision number was specified as outlined here:
https://fedoraproject.org/wiki/Packaging/NamingGuidelines#SnapshotPackages
I added the svn comment above the Source tag rather than a wget at the top.
* Some BR are not required:
https://fedoraproject.org/wiki/Packaging:Guidelines#Exceptions_2
They were specified more to remind myself I need them when using a live CD to
test a clean build. I removed them if they are troublesome to include.
* Do not strip binaries! Otherwise the debuginfo package is useless:
https://fedoraproject.org/wiki/Packaging/Debuginfo
I forgot to install redhat-rpm-config at first, so rpmlint was angry at the
unstripped files. This is fixed.
* Avoid using dos2unix. Use sed instead:
https://fedoraproject.org/wiki/Packaging:Guidelines#Rpmlint_Errors
Done.
* I see that you use the %{__xxx} macro style. Even though this is
not a bug
and it is not required that you change it, I suggest you not to use this style
because it is not very readable. I cannot think of a good reason to use it
instead of just using "xxx".
I removed the macros.
* The icon cache scriptlets are not the same used by the Fedora
guidelines:
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets#Icon_Cache
I didn't know they were required to be the same; I'll use their version now.
I haven't checked every requirements but the spec file is quite
good as a first
release from a rookie :)
Unluckily I'm not a sponsor and therefore I cannot sponsor you. I suggest you
to do "informal" reviews of other packages to show your understanding of
Fedora
guidelines to potential sponsors.
Thanks for the corrections. Let me know if you notice anything else. I will
have a look around other review requests.
Updated Spec: http://dave.is1337.net/sources/vbam.spec
Updated SRPM:
http://dave.is1337.net/sources/vbam-1.8.0-0.2.20090316svn856.fc10.src.rpm
--
Configure bugmail: http://bugzilla.rpmfusion.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.
You are the assignee for the bug.