Note that I am not yet a sponsored packager. Spec URL: http://wfp.fedorapeople.org/perl-XML-Rules.spec SRPM URL: http://wfp.fedorapeople.org/perl-XML-Rules-1.10-1.fc14.src.rpm Description: The XML::Rules module provides an API layer on top of XML::Parser. It allows you to specify rules that are subroutines to be run once a tag is fully parsed and either process the data from the tag itself and its children or specify what parts of the data and how to add to the data structure being built for the parent tag.
rpmlint output: perl-XML-Rules.noarch: W: file-not-utf8 /usr/share/doc/perl-XML-Rules-1.10/example/testUTF.txt perl-XML-Rules.noarch: W: file-not-utf8 /usr/share/doc/perl-XML-Rules-1.10/example/testUTF.pl perl-XML-Rules.noarch: W: file-not-utf8 /usr/share/doc/perl-XML-Rules-1.10/example/test19.txt perl-XML-Rules.noarch: W: file-not-utf8 /usr/share/doc/perl-XML-Rules-1.10/example/test18.xml These files are either explicitly encoded using some ither encoding, or are generated from such a file, so these warnings can be dismissed. - package and spec file name as per guidelines - package meets guidelines - license is OK for Fedora and matches upstream - upstream license text included in package - spec file written in English and is legible - source matches upstream - builds OK in mock for Rawhide x86_64 - buildreqs look OK - no locales, libraries, devel files etc. to concern us - package is not intended to be relocatable - directory ownership is fine - no duplicate files - no permissions problems - macro usage is consistent - code, not content - no large docs - docs don't affect runtime - not a GUI application - all filenames OK - no scriptlets - requires/provides are OK - test suite passes cleanly - package installs cleanly Requires perl >= 0:5.008 perl(:MODULE_COMPAT_5.12.3) perl(Carp) perl(Exporter) perl(Scalar::Util) perl(XML::Parser) >= 2.00 perl(XML::Parser::Expat) perl(XML::Parser::Expat) >= 2.00 perl(constant) perl(strict) perl(warnings) rpmlib(CompressedFileNames) <= 3.0.4-1 rpmlib(FileDigests) <= 4.6.0-1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1 rpmlib(PayloadIsXz) <= 5.2-1 Provides perl(XML::Rules) = 1.10 perl-XML-Rules = 1.10-1.fc16 Quibbles - the blank line after %description is treated as part of the package description, so it's best not to have it there - it's best not to assume that RPM will use gzip to compress manpages as this may change in the future; better change: %{_mandir}/man3/XML::Rules.3pm.gz to: %{_mandir}/man3/XML::Rules.3pm* - I'd suggest shipping the two scripts you removed from %{_bindir} as %doc - do you intend to build these packages for EPEL-4 or EPEL-5? If not, you can skip cleaning the buildroot in %install, skip %clean altogether and drop the BuildRoot: tag
package and spec updated to address the quibbles. I use this rpm locally on EPEL-5, so yes, I'd expect to build it for EPEL-5 repo. Spec URL: http://wfp.fedorapeople.org/perl-XML-Rules.spec SRPM URL: http://wfp.fedorapeople.org/perl-XML-Rules-1.10-2.fc14.src.rpm
Hi Bill, I'd like to get this moving again; a couple of days ago I used this module in anger and I think it's great! I'd forgotten all about this review request so I cobbled together a package myself. Anyway, a few more comments: * I'd suggest using a patch to add perl shellbangs to the dtd2XMLRules.pl and xml2XMLRules.pl scripts in %prep and then just ship them in %{_bindir} as upstream intended. * Consider packaging XML::DTDParser, which is needed by inferRulesFromDTD; if you do so, add it as an explicit dependency of this package. * You can drop the redundant buildreq perl(version). * Consider dropping the explicit versioned Requires: for perl(XML::Parser) and perl(XML::Parser::Expat); even RHEL-3 had sufficiently recent versions of these. * Consider adding BuildRequires: for perl(Carp), perl(constant), perl(Exporter) and perl(Scalar::Util), all of which live on CPAN and might be dual-lived as separate packages in Fedora. Cheers, Paul.
Here's an update that addresses the comments, minus packaging XML::DTDParser -- I'll package that up shortly. Spec URL: http://wfp.fedorapeople.org/perl-XML-Rules.spec SRPM URL: http://wfp.fedorapeople.org/perl-XML-Rules-1.10-3.fc16.src.rpm
You should add the shellbangs (and make any other changes to the sources) in %prep rather than %install unless there's a good reason not to. I think it would be more readable if you had each of the new buildreqs on a separate line, like the existing ones. You can drop the "Requires:" lines for perl(XML::Parser) and perl(XML::Parser::Expat) altogether since rpm finds the dependencies itself and the versions don't matter as discussed previously. Let me know when you have an XML::DTDParser package to review; we should be able to move forward with the sponsorship process then and get these things into Fedora.
Another update to address previous comment. Spec URL: http://wfp.fedorapeople.org/perl-XML-Rules.spec SRPM URL: http://wfp.fedorapeople.org/perl-XML-Rules-1.10-4.fc16.src.rpm I'll make a separate review request for XML::DTDParser
New Package SCM Request ======================= Package Name: perl-XML-Rules Short Description: Parse XML and specify what and how to keep/process for individual tags Owners: wfp Branches: f16 f17 InitialCC:
Bill, please await package approval before making an SCM request. Thanks!
Hopefully I'll have time to look at this tomorrow.
An updated version of this, fixing a build problem I discovered. Spec URL: http://wfp.fedorapeople.org/perl-XML-Rules.spec SRPM URL: http://wfp.fedorapeople.org/perl-XML-Rules-1.10-5.fc16.src.rpm
Please fix the line endings in %prep rather than %build.
end of line encoding moved to %prep stage. Spec URL: http://wfp.fedorapeople.org/perl-XML-Rules.spec SRPM URL: http://wfp.fedorapeople.org/perl-XML-Rules-1.10-6.fc16.src.rpm
perl-XML-Rules review ===================== rpmlint output: perl-XML-Rules.noarch: E: incorrect-fsf-address /usr/share/doc/perl-XML-Rules-1.10/LICENSE perl-XML-Rules.noarch: W: file-not-utf8 /usr/share/doc/perl-XML-Rules-1.10/example/testUTF.txt perl-XML-Rules.noarch: W: file-not-utf8 /usr/share/doc/perl-XML-Rules-1.10/example/testUTF.pl perl-XML-Rules.noarch: W: file-not-utf8 /usr/share/doc/perl-XML-Rules-1.10/example/test19.txt perl-XML-Rules.noarch: W: file-not-utf8 /usr/share/doc/perl-XML-Rules-1.10/example/test18.xml perl-XML-Rules.noarch: W: no-manual-page-for-binary xml2XMLRules.pl perl-XML-Rules.noarch: W: no-manual-page-for-binary dtd2XMLRules.pl You should contact upstream and suggest they update the GPL license text in any future release to fix the address (they can download this text from http://www.gnu.org/licenses/old-licenses/gpl-1.0.txt). The non-UTF8 files are explicitly using other encodings, so that can be ignored. This is quite common with XML-handling packages. The scripts without manpages are small and self-explanatory so I don't think there's anything to worry about there. - package and spec naming OK - package meets guidelines - license matches upstream and is OK for Fedora - upstream LICENSE file included in package - spec written in English and is legible - source tarball content matches upstream - package builds and runs OK on Fedora - buildreqs comprehensive and appropriate - no locale data or libraries to worry about - package is not intended to be relocatable - file and directory ownership OK - no duplicate files - file and directory permissions sane - macro usage inconsistent - see below - code, not content - no large documentation - docs don't affect runtime - no devel-type content to worry about - not a GUI app so no desktop file needed - filenames all plain ASCII - no scriptlets or subpackages to worry about Suggestions: My preference for handling addition/removal of shellbangs (or re-coding of documentation to UTF-8) is to use a patch rather than running sed (or iconv) in %prep. The reason for this is that future updates from upstream may include the shellbangs or have been recoded to UTF8 there, and if you don't notice that, you can end up with two shellbangs (ugly but harmless), or mangled documentation in the iconv case. Not a blocker though. The timestamp of the source tarball is more recent than the upstream tarball, though the content is the same. It would be neater to re-download it prior to putting in the Fedora lookaside cache so that the timestamp would match. You've used both "sed" and "%{__sed}" in the spec, which is inconsistent. You should just use "sed" - see http://fedoraproject.org/wiki/Packaging:Guidelines#Macros You can fix that in git before building the package. APPROVED, assuming you're going to fix the macro usage.
New Package SCM Request ======================= Package Name: perl-XML-Rules Short Description: Parse XML and specify what and how to keep/process for individual tags Owners: wfp Branches: f16 f17 InitialCC: perl-sig
Git done (by process-git-requests).
Bill, you only seem to have done a build for Rawhide; is there some problem with doing the builds for f16/f17?
No problems, just me being new to the process and missing those steps. I'll get on it now.
perl-XML-Rules-1.10-7.fc16 has been submitted as an update for Fedora 16. https://admin.fedoraproject.org/updates/perl-XML-Rules-1.10-7.fc16
perl-XML-Rules-1.10-7.fc17 has been submitted as an update for Fedora 17. https://admin.fedoraproject.org/updates/perl-XML-Rules-1.10-7.fc17
perl-XML-Rules-1.10-7.fc17 has been pushed to the Fedora 17 testing repository.
perl-XML-Rules-1.10-7.fc16 has been pushed to the Fedora 16 stable repository.
perl-XML-Rules-1.10-7.fc17 has been pushed to the Fedora 17 stable repository.