2:47 p.m.
http://bugzilla.rpmfusion.org/show_bug.cgi?id=15
--- Comment #43 from Julian Sikorski <belegdol(a)gmail.com> 2008-11-30 15:47:11 ---
(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 ?
--
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.