[Bug 15] Review Request: bsnes - SNES emulator focused on accuracy

http://bugzilla.rpmfusion.org/show_bug.cgi?id=15 --- Comment #42 from David Timms <dtimms(a)iinet.net.au&gt; 2008-11-30 15:19:42 --- (In reply to comment #41) Done as requested. Full review: [ OK] rpmlint is clean: $ rpmlint /home/davidt/rpmbuild/SRPMS/bsnes-0.037a-4.fc9.src.rpm /home/davidt/rpmbuild/RPMS/i386/bsnes-0.037a-4.fc9.i386.rpm /home/davidt/rpmbuild/RPMS/i386/bsnes-debuginfo-0.037a-4.fc9.i386.rpm 3 packages and 0 specfiles checked; 0 errors, 0 warnings. [ OK] name meets guidelines [ OK] spec filename is correct [ OK] meets package quidelines [ OK] license is non-free => rpmfusion not fedora [ OK] license field matches upstream redistributable, no mods. [ OK] license text file is included [ OK] spec file is legible, easy to understand [ x ] source URL is incorrect: think it should be: ...%{name}_v0.%{vernumber}... upstream calls it 037a but compresses source to v0.037a [ OK] upstream md5sum matches .src.rpm $ md5sum bsnes_v037a.tar.bz2 9fa2fbae8a09a747f9b4a44123bde0bb bsnes_v037a.tar.bz2 $ md5sum bsnes-0.037a-4.fc10.src/bsnes_v037a.tar.bz2 9fa2fbae8a09a747f9b4a44123bde0bb bsnes-0.037a-4.fc10.src/bsnes_v037a.tar.bz2 [ OK] rpmbuild binary rpms built. [ x ] ExcludeArch not used in favour of ExclusiveArch: i386 x86_64 is there no possibility of this working on ppc etc. If so, I believe a comment above this field to briefly indicate why it's x86 only, could be "application uses x86 CPU machine code, so needs an x86 CPU" {if that is the reason}. [Not sure if we want to make a bugzilla/block tracker bug - depends on the reason why no ppc/other architectures] [ OK] BuiildRequires are specified. [ ? ] No localization is provided in the main source. I am not sure what work would be required to use the available individual locale.cfg files automatically. It seems you would simply copy a single one of them into the bin dir as locale.cfg, and bsnes would only use that translation ? [ NA] no shared libraries are compiled or installed [ OK] No relocatable (Prefix: is not used) [ OK] owns directories it creates, disowns other dirs. [ OK] no duplicates in the %files stanza [ OK] permissions defattr included, other files have their perms fixed in %prep [ OK] %clean matches required content [ OK] code versus content: this is a hardware emulator, no roms included. [ OK] doc is small -> no separate -doc package [ OK] no %doc files are not required at runtime [ NA] no header files, no static libraries, no pkgconfig, no libaries, no -devel, no libtool archives. [ OK] gui app and has legit .desktop file, installed by desktop-file-install, and BRs: desktop-file-utils [ OK] source filenames appear to be valid utf-8 [ OK] package functions as described. [ ? ] an icon provided at /usr/share/icons/, yet the GTK+ icon cache update scriptlet isn't used - why not ? why was it removed when it was previously included ? [ ? ] (In reply to comment #8) my opinion on use of included rather than system library for snes_ntsc is it would be a lot of effort, and the included version calculates the values differently, so we would need to decide if those changes could get pushed upstream. Also given byuu's opinion on the zlib that you worked on already, it would be unlikely to be included upstream. Julian: I think we are really close here, just a few items above {x and ?} to either fix or provide reasoning for. Reading back over the review, it seems you have completed pretty well all requests reviewers have made, beside the "make it use all system libaries". If there are other reviewers who believe further work should be done on the "use system libraries" item, can you please speak up now ? -- 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.