(In reply to comment #42) > (In reply to comment #41) > > Spec URL: http://belegdol.republika.pl/rpmstuff/bsnes.spec > > SRPM URL: http://belegdol.republika.pl/rpmstuff/bsnes-0.037a-4.fc10.src.rpm > > > > Changes: > > - Fixed README.Fedora permissions > > - Added information concerning pulseaudio issues > 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 I'm not sure what you mean here, that's how the upstream tarball is called (I'm aware it uncompresses to a folder of slightly different name. Besides, if the Source0 URL would be incorrect, the package won't build, would it?

> [ 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, I'll add an explanation then. The reason is that libco only supports these two 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 ? AFAIK, bsnes can use only one locale.cfg at a time, so packaging several at once seems impossible. > [ 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 ? It was removed because the icon was installed to datadir/pixmaps at the time upstream was not installing it at all. I pointed out to byuu that datadir/icons is a place for themed icons and bsnes one should go to datadir/pixmaps, but that was left without response. Also, packaging guidelines are only saying about subdirs of datadir/icons. > [ ? ] (In reply to comment #8) > > It seems that bsnes is using its own versions of system libraries. This should > > be avoided. > 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 ? >