No vendor or packager fields needed: please remove.

Is there a web page for the package that could go in the url field?
Maybe something like http://search.cpan.org/dist/ParseLex?

It would be nice to use caps for the rpm fields: eg "Name", "Version", etc.

%NVR is unused.

make test should be move to a %check section and then %maketest would not be needed.

(In reply to comment #1)

I've updated all of these in the spec file and checked it in. The SRPM hasn't
been updated yet/

> No vendor or packager fields needed: please remove.
>
> Is there a web page for the package that could go in the url field?
> Maybe something like http://search.cpan.org/dist/ParseLex?
> 
> It would be nice to use caps for the rpm fields: eg "Name", "Version", etc.
> 
> %NVR is unused.
> 
> make test should be move to a %check section and then %maketest would not be
needed.

Please make yourself familiar with the Fedora Packaging Guidelines.
http://fedoraproject.org/wiki/Packaging/Guidelines

Needswork:

- Package is noarch
=> remove OPTIMIZE="$RPM_OPT_FLAGS" from main make call

- Remove the %doc from the mans in %files
mans are automatically %doc'ed by rpm

- This construct is unnecessarily complex:
%dir %{perl_vendorlib}/Parse
%{perl_vendorlib}/Parse/*.pm

%{perl_vendorlib}/Parse
would suffice

- The 
chmod -R u+w %{_builddir}/%{pkgname}-%{version}
in %prep is superfluous, remove it.

- The construct as being used in %clean is being frowned upon in fedora.
Use
%clean
rm -rf %{buildroot}
instead.

- Your %buildroot doesn't comply to the packaging guidelines.

- The %defines at the beginning of the spec are unused. Remove them.

http://svn.fedorahosted.org/svn/documentation-devel/trunk/Files/perl-ParseLex.spec
http://svn.fedorahosted.org/svn/documentation-devel/trunk/Files/perl-ParseLex-2.15-7.fc9.noarch.rpm

http://svn.fedorahosted.org/svn/documentation-devel/trunk/Files/perl-ParseLex-2.15-7.fc9.src.rpm

Some additional comments:

This package doesn't build in mock against rawhide; it is missing a build
dependency on perl(ExtUtils::MakeMaker).

The tarball includes a test suite, but the spec doesn't call it.  And
unfortunately, when I add a check section to call it, test4 fails.  I think the
test is has simply been broken by some perl update over the past nine years; the
difference in the tests is:

< can't analyze: "this is an invalid string with a "" in it"" at
examples/ctokenizer.pl line 17, <DATA> line 4.
---
> can't analyze: "this is an invalid string with a "" in it"" at
examples/ctokenizer.pl line 17, <DATA> chunk 4.

So s/chunk/line/ in the expected data from the test and it should pass.

rpmlint says the following:
  perl-ParseLex.noarch: W: file-not-utf8 
   /usr/share/doc/perl-ParseLex-2.15/Changes
This should be passed through iconv; see the example in 
http://fedoraproject.org/wiki/PackageMaintainers/CommonRpmlintIssues

  perl-ParseLex.noarch: E: useless-explicit-provides perl(Parse::Token)
This is more interesting; rpm extracts the following two dependencies (in
addition to several others, of course):
  perl(Parse::Token)
  perl(Parse::Token) = 2.15

This comes from the "package Parse::Token;" lines in Token.pm and Token-t.pm. 
Unfortunately rpm screws up sometimes and these errant dependencies need to be
filtered.  If you were using cpanspec to generate your specfile then this comes
for free with --filter-provides but since you've rolled your own, take a look at
the examples at http://fedoraproject.org/wiki/Packaging/Perl.

* Wed Jan 16 2008 Jeff Fearn <jfearn@redhat.com> 2.15-8
- Changed Development/Languages to Development/Libraries
- Fixed test
- Removed useless-explicit-provides
- Converted Changes to utf-8

http://svn.fedorahosted.org/svn/documentation-devel/trunk/Files/perl-ParseLex.spec
http://svn.fedorahosted.org/svn/documentation-devel/trunk/Files/perl-ParseLex-2.15-8.fc9.src.rpm

http://svn.fedorahosted.org/svn/documentation-devel/trunk/Files/perl-ParseLex-2.15-9.fc9.src.rpm
http://svn.fedorahosted.org/svn/documentation-devel/trunk/Files/perl-ParseLex.spec

Missed that BuildRequires <blush>

I'll do a full review....

Builds OK for me.  rpmlint is down to just
  perl-ParseLex.noarch: E: useless-explicit-provides perl(Parse::Token)
I see where you added the filter, but I think you neglected to change something on the sed line.

What you need is:
  cat << \EOF > ParseLex-prov
  #!/bin/sh
  %{__perl_provides} $* |\
    sed -e '/perl(Parse::Token)$/d'
  EOF
Note the change from "unwanted_provides" to "Parse::Token", but also note that I've anchored the match to the end of the string so the proper versioned provide doesn't get removed as well.

I notice that the tarball included in the srpm is not the same as the tarball on CPAN.  Do you know what's going on there?  Upstream source hasn't changed in nine years but the tarball in the srpm has a modification time of the 15th.

If you are going to use macros like %{__make}, you should consistently use macros.  Which means %{__rm} in %clean and %{__cat}, %{__sed}, %{__chmod} in %prep. (There's no macro for find for whatever reason.)  Or you can just drop the macros.  It's up to your personal preference, but you must be consistent.

The Summary: is a bit content-free (it's just the name turned around with module appended).  Upstream says "Generator of lexical analyzers" which at least says something about what it does.

Checklist:
X source files don't match upstream.
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written
X specfile does not use macros consistently.
X summary would use some improvement.
* description is OK.
* dist tag is present.
* build root is OK.
* license field matches the actual license.
* license is open source-compatible.
* license text not included upstream.
* latest version is being packaged.
* BuildRequires are proper.
* %clean is present.
* package builds in mock (rawhide, x86_64).
* package installs properly
X rpmlint has a valid complaint.
X still an errant provide:
   perl(Parse::ALex) = 2.15
   perl(Parse::CLex)
   perl(Parse::Lex)
   perl(Parse::LexEvent) = 1.00
   perl(Parse::Template) = 0.32
X  perl(Parse::Token)
   perl(Parse::Token) = 2.15
   perl(Parse::Token::Action)
   perl(Parse::Token::Delimited)
   perl(Parse::Token::Nested)
   perl(Parse::Token::Quoted)
   perl(Parse::Token::Segmented)
   perl(Parse::Token::Simple)
   perl(Parse::Tokenizer)
   perl(Parse::Trace)
   perl(Parse::YYLex) = 0.91
   perl-ParseLex = 2.15-9.fc9
  =
   perl >= 0:5.000
   perl >= 0:5.003
   perl >= 0:5.004
   perl >= 0:5.005
   perl(Carp)
   perl(Parse::ALex)
   perl(Parse::Lex)
   perl(Parse::Template)
   perl(Parse::Token) >= 2.15
   perl(Parse::Trace)
   perl(constant)
   perl(integer)
   perl(strict)
   perl(vars)

* %check is present and all tests pass:
   All tests successful.
   Files=7, Tests=7,  1 wallclock secs ( 0.17 cusr +  0.06 csys =  0.23 CPU)

* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no scriptlets present.
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.

(In reply to comment #9)
> I'll do a full review....
> 
> Builds OK for me.  rpmlint is down to just
>   perl-ParseLex.noarch: E: useless-explicit-provides perl(Parse::Token)
> I see where you added the filter, but I think you neglected to change
something on the sed line.
> 
> What you need is:
>   cat << \EOF > ParseLex-prov
>   #!/bin/sh
>   %{__perl_provides} $* |\
>     sed -e '/perl(Parse::Token)$/d'
>   EOF
> Note the change from "unwanted_provides" to "Parse::Token", but also note that
I've anchored the match to the end of the string so the proper versioned provide
doesn't get removed as well.

Fixed.

> I notice that the tarball included in the srpm is not the same as the tarball
on CPAN.  Do you know what's going on there?  Upstream source hasn't changed in
nine years but the tarball in the srpm has a modification time of the 15th.

As per #6 the Changes file was converted to UTF8 and test 4 was tweaked for
newer test modules.

These two changes + ParseLex-2.15-syntax.patch need to be pushed upstream, time
limitations are the only reason they haven't been.

> If you are going to use macros like %{__make}, you should consistently use
macros.  Which means %{__rm} in %clean and %{__cat}, %{__sed}, %{__chmod} in
%prep. (There's no macro for find for whatever reason.)  Or you can just drop
the macros.  It's up to your personal preference, but you must be consistent.

Consistorised ... if it's not a word it should be :)

> The Summary: is a bit content-free (it's just the name turned around with
module appended).  Upstream says "Generator of lexical analyzers" which at least
says something about what it does.

Updated.

http://svn.fedorahosted.org/svn/documentation-devel/trunk/Files/perl-ParseLex.spec
http://svn.fedorahosted.org/svn/documentation-devel/trunk/Files/perl-ParseLex-2.15-10.fc9.src.rpm

It's not acceptable to modify the upstream tarball.  You must take a pristine
tarball from upstream, untar it, and make your changes via patching or
sed/perl/iconv/whatever in %prep.  But the tarball that's in the SRPM must be
identical to the tarball I can download from the Source: URL.  (There are
certain exceptions to this such as when the upstream tarball contains something
we can't legally distribute, but this is not one of those cases.)  Also, note
that upstream hasn't updated this package in nine years; I think the chances are
slim that you'll be able to push patches to them.

At this point the package looks OK, but I simply cannot approve it with a
modified tarball.

* Fri Jan 18 2008 Jeff Fearn <jfearn@redhat.com> 2.16-0
- Become upstream

http://svn.fedorahosted.org/svn/documentation-devel/trunk/Files/perl-ParseLex.spec
http://svn.fedorahosted.org/svn/documentation-devel/trunk/Files/perl-ParseLex-2.16-0.fc9.src.rpm

New Source file:
http://svn.fedorahosted.org/svn/documentation-devel/trunk/Files/ParseLex-2.16.tar.gz

Isn't that a bit extreme?  Have you communicated with upstream about this?  I
mean, forking the package to avoid one call to iconv and a single patch seems a
bit over the top.

It's an easy solution that allows more pressing issue to be attended to. When
there is less time pressure I will get these changes upstream. Until then I am
the upstream for this package.

That I can't go along with; it's just not the proper way to do things in Fedora.
 Sorry.  Returning this ticket to the review queue.

http://svn.fedorahosted.org/svn/documentation-devel/trunk/Files/perl-ParseLex.spec
http://svn.fedorahosted.org/svn/documentation-devel/trunk/Files/perl-ParseLex-2.15-11.fc9.src.rpm

Someone pinged me to let me know that you had updated this package, so I went
ahead and had a look and I'm pleased to say that this package is fine.  Thanks
for taking the effort to understand how we do things in Fedora.

* source files now match upstream:
   a6201f0522b36733ec237e37ff8328f306b23081ea8473de7456d18831d3e997  
   ParseLex-2.15.tar.gz
* specfile now uses macros consistently.
* summary is OK.
* rpmlint is silent.
* The errant provide is gone.

APPROVED

Please let me know if you need assistance with the rest of the process.

New Package CVS Request
=======================
Package Name: perl-ParseLex
Short Description: Perl lexical analyzer
Owners: jfearn
Branches: F-8 EL-4 EL-5
InitialCC: mdious
Cvsextras Commits: yes

cvs admin done

(In reply to comment #19)
> cvs admin done

$ cvs co perl-ParseLex
cvs server: cannot find module `perl-ParseLex' - ignored
cvs [checkout aborted]: cannot expand modules

$ cvs -f co rpms/perl-ParseLex
cvs checkout: warning: new-born rpms/perl-ParseLex has disappeared

$ cvs exteminate --target gremlins --force extreeme
cvs exterminate: too many gremlins, getting the hell out of here!

I saw it in pkgdb, but I couldn't find it in CVS at all.  I went ahead and did
an addpackage and it seems to be there now.

yay, it's all working now, thanks.

Built OK.

http://koji.fedoraproject.org/koji/taskinfo?taskID=371350