Per talking with tibbs on IRC I am going to take over the review, as I had just 
started in on one just before he did. 

OK - Package name
OK - Spec file matches base package name.
OK - Meets Packaging Guidelines.
OK - License (LGPL)
OK - License field in spec matches
OK - License file included in package
OK - Spec in American English
OK - Spec is legible.
OK - Sources match upstream md5sum:
ab7fb92a1ed0db54a88839e64b9ce2c6  SDL_perl-1.20.3.tar.gz
ab7fb92a1ed0db54a88839e64b9ce2c6  SDL_perl-1.20.3.tar.gz.1
OK - Package compiles and builds on at least one arch.
n/a - Package needs ExcludeArch
OK - BuildRequires correct
n/a - Spec handles locales/find_lang
n/a - Spec has needed ldconfig in post and postun
n/a - Package is relocatable and has a reason to be.
OK - Package owns all the directories it creates.
OK - Package has no duplicate files in %files.
OK - Package has %defattr and permissions on files is good.
OK - Package has a correct %clean section.
OK - Spec has consistant macro usage.
OK - Package is code or permissible content.
n/a - -doc subpackage needed/used.
OK - Packages %doc files don't affect runtime.
n/a - Headers/static libs in -devel subpackage.
n/a - .pc files in -devel subpackage.
n/a - .so files in -devel subpackage.
n/a - -devel package Requires: %{name} = %{version}-%{release}
n/a - .la files are removed.
n/a - Package is a GUI app and has a .desktop file
OK - Package doesn't own any directories other packages own.
OK - No rpmlint output.

SHOULD Items:

OK - Should include License or ask upstream to include it.
OK - Should build in mock.

Issues/Questions:

1. There is a SDL_perl 2.1.3 on CPAN:
http://search.cpan.org/~dgoehrig/SDL_Perl-2.1.3/
Is that version usable for the packages that use this version?
Or totally diffrent interface? If that package is imported someday
would it conflict with this one?

(In reply to comment #1)
> Issues/Questions:
> 
> 1. There is a SDL_perl 2.1.3 on CPAN:
> http://search.cpan.org/~dgoehrig/SDL_Perl-2.1.3/
> Is that version usable for the packages that use this version?
> Or totally diffrent interface? 

I haven't tried myself but have been told by the previous maintainers from the
repo that must not be named that that version is not usable, so I assumed it has
a different interface. Also the frozen-bubble package which this bug block
contains: "Requires:       perl-SDL >= 0:1.19.0, perl-SDL < 0:2.0" and an
identical BR.

However I've just checked a few other distros / rpm-repo's and I've found that
rpmforge have frozen-bubble working with perl-SDL 2.1.2 (with a small patch, so
I guess the interface really is different).


> If that package is imported someday
> would it conflict with this one?
> 

I honestly don't know, but since it seems that frozen-bubble can be made to work
with 2.1.x quite easily I'll guess it would be better to make the jumpt to 2.1.x
now. I'll post a new version soon.

Ok, I've updated both perl-SDL and frozen-bubble to work with the newer perl-SDL.
New perl-SDL here:
Spec URL: http://people.atrpms.net/~hdegoede/perl-SDL.spec
SRPM URL: http://people.atrpms.net/~hdegoede/perl-SDL-2.1.3-1.src.rpm

p.s.

I think this should only be build for the devel branch now since it breaks
existing apps, which is BAD todo with release versions. Leaving it in that other
repo for FC-4 and FC-5.

The version from comment #3 fails in mock here with (from build.log):
+ echo 'Patch #0 (sdlperl_2.1.2-1.diff.gz):'
Patch #0 (sdlperl_2.1.2-1.diff.gz):
+ /bin/gzip -d
+ patch -p1 -s
+ STATUS=0
+ '[' 0 -ne 0 ']'
+ patch -p1 -b -z .deb
patching file src/OpenGL.xs
+ patch -p1
patching file src/OpenGL.xs
Hunk #1 succeeded at 912 (offset 2 lines).
Hunk #3 succeeded at 1006 (offset 2 lines).
Hunk #5 succeeded at 1090 (offset 2 lines).
+ echo 'Patch #1 (perl-SDL-no-mixertest.patch):'
Patch #1 (perl-SDL-no-mixertest.patch):
+ patch -p1 -b --suffix .no-mixertest -s
The text leading up to this was:
--------------------------
|--- t/mixerpm.t~       2003-03-21 19:48:58.000000000 +0200
|+++ t/mixerpm.t        2004-07-11 00:12:59.851670873 +0300
--------------------------
File to patch:
Skip this patch? [y]
2 out of 2 hunks ignored
error: Bad exit status from /var/tmp/rpm-tmp.2893 (%prep)

Agreed with comment #4. 

(In reply to comment #5)
> The version from comment #3 fails in mock here with (from build.log):
> + echo 'Patch #0 (sdlperl_2.1.2-1.diff.gz):'
> Patch #0 (sdlperl_2.1.2-1.diff.gz):
> + /bin/gzip -d
> + patch -p1 -s
> + STATUS=0
> + '[' 0 -ne 0 ']'
> + patch -p1 -b -z .deb
> patching file src/OpenGL.xs
> + patch -p1
> patching file src/OpenGL.xs
> Hunk #1 succeeded at 912 (offset 2 lines).
> Hunk #3 succeeded at 1006 (offset 2 lines).
> Hunk #5 succeeded at 1090 (offset 2 lines).
> + echo 'Patch #1 (perl-SDL-no-mixertest.patch):'
> Patch #1 (perl-SDL-no-mixertest.patch):
> + patch -p1 -b --suffix .no-mixertest -s
> The text leading up to this was:
> --------------------------
> |--- t/mixerpm.t~       2003-03-21 19:48:58.000000000 +0200
> |+++ t/mixerpm.t        2004-07-11 00:12:59.851670873 +0300
> --------------------------
> File to patch:
> Skip this patch? [y]
> 2 out of 2 hunks ignored
> error: Bad exit status from /var/tmp/rpm-tmp.2893 (%prep)
> 
> Agreed with comment #4. 

Erm, a patch failing to apply is not mock specific, it seems that for some
reason you are still using the patch from the 1.20.3 package which was updated
for / in the 2.1.3 package. I just tried to install the provided SRPM on a
different PC then where it was developed and there it builds fine.

ugh. I thought I checked all the files, but I missed that patch, you are right. 
Sorry for the trouble...

Since this is pretty much a totally new package,
I will go ahead and run thru my review checklist on it
again. ;)

OK - Package name
OK - Spec file matches base package name.
OK - Meets Packaging Guidelines.
OK - License (LGPL)
OK - License field in spec matches
OK - License file included in package
OK - Spec in American English
OK - Spec is legible.
OK - Sources match upstream md5sum:
6ce26e1b710ce52def4ec22637cd5176  SDL_Perl-2.1.3.tar.gz
6ce26e1b710ce52def4ec22637cd5176  SDL_Perl-2.1.3.tar.gz.1
OK - Package compiles and builds on at least one arch.
n/a - Package needs ExcludeArch
OK - BuildRequires correct
n/a - Spec handles locales/find_lang
n/a - Spec has needed ldconfig in post and postun
n/a - Package is relocatable and has a reason to be.
OK - Package owns all the directories it creates.
OK - Package has no duplicate files in %files.
OK - Package has %defattr and permissions on files is good.
OK - Package has a correct %clean section.
OK - Spec has consistant macro usage.
OK - Package is code or permissible content.
n/a - -doc subpackage needed/used.
OK - Packages %doc files don't affect runtime.
n/a - Headers/static libs in -devel subpackage.
n/a - .pc files in -devel subpackage.
n/a - .so files in -devel subpackage.
n/a - -devel package Requires: %{name} = %{version}-%{release}
n/a - .la files are removed.
n/a - Package is a GUI app and has a .desktop file
OK - Package doesn't own any directories other packages own.
See below - No rpmlint output.

SHOULD Items:

OK - Should include License or ask upstream to include it.
OK - Should build in mock.

Issues:

1. rpmlint says:

W: perl-SDL strange-permission filter-depends.sh 0755

I think that can be ignored. It should be 755 I would think.

2. I see the package is providing:
perl(Walker)
and
perl(main)
Is that correct or should those be filtered out as well?

(In reply to comment #7)
> ugh. I thought I checked all the files, but I missed that patch, you are right. 
> Sorry for the trouble...
>
No problem.
 
> 1. rpmlint says:
> 
> W: perl-SDL strange-permission filter-depends.sh 0755
> 
> I think that can be ignored. It should be 755 I would think.
>
Yes very much so.

> 2. I see the package is providing:
> perl(Walker)
> and
> perl(main)
> Is that correct or should those be filtered out as well?
> 

That indeed looks wrong I'll do a new version filtering out those too.

Version without the wrong Provides:
Spec URL: http://people.atrpms.net/~hdegoede/perl-SDL.spec
SRPM URL: http://people.atrpms.net/~hdegoede/perl-SDL-2.1.3-2.src.rpm

Excellent. That looks all good to go to me, so this package is APPROVED. 
Don't forget to close this as NEXTRELEASE when you have it checked in and built 
for devel. 

Thanks I'll import it right away but I'll wait with the building until
frozen-bubble is approved too, so I can build them in quick succession in order
to not brake anybody's systems.

Build (finally), closing.