Bug 473215 (perl-HTML-FormFu) - Review Request: perl-HTML-FormFu - HTML Form Creation, Rendering and Validation Framework
Summary: Review Request: perl-HTML-FormFu - HTML Form Creation, Rendering and Validati...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: perl-HTML-FormFu
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: manuel wolfshant
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-11-27 06:14 UTC by Iain Arnell
Modified: 2009-01-07 09:21 UTC (History)
4 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2008-12-05 15:37:49 UTC
Type: ---
Embargoed:
manuel.wolfshant: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Iain Arnell 2008-11-27 06:14:38 UTC
Spec URL: http://iarnell.fedorapeople.org/perl-HTML-FormFu.spec
SRPM URL: http://iarnell.fedorapeople.org/perl-HTML-FormFu-0.03005-1.fc11.src.rpm
Description: 
HTML::FormFu is a HTML form framework which aims to be as easy as possible
to use for basic web forms, but with the power and flexibility to do
anything else you might want to do (as long as it involves forms).

Comment 1 manuel wolfshant 2008-11-29 13:25:09 UTC
Could you please review the list of perl modules explicitly required? As far as I can see the rpmbuild depchecker picks some of the requirements that you have chosen to add. For this reason "rpm -qp --provides" lists a lot of duplicates:

[wolfy@wolfy ~]$ rpm -qp --requires perl-HTML-FormFu-0.03005-1.fc11.noarch.rpm
/usr/bin/perl
perl(:MODULE_COMPAT_5.10.0)
perl(Captcha::reCAPTCHA)
perl(Captcha::reCAPTCHA) >= 0.92
perl(Carp)
perl(Class::Accessor::Chained::Fast)
perl(Class::C3)
perl(Class::C3)
perl(Config::Any)
perl(Config::Any) >= 0.10
perl(Crypt::CBC)
perl(Crypt::DES)
perl(Cwd)
perl(Data::Dumper)
perl(Data::Visitor::Callback)
perl(Data::Visitor::Callback)
perl(Date::Calc)
perl(DateTime)
perl(DateTime) >= 0.38
perl(DateTime::Format::Builder)
perl(DateTime::Format::Builder) >= 0.7901
perl(DateTime::Format::Natural)
perl(DateTime::Format::Natural)
perl(DateTime::Format::Strptime)
perl(DateTime::Format::Strptime)
perl(DateTime::Locale)
perl(DateTime::Locale)
perl(Email::Valid)
perl(Email::Valid)
perl(Encode)
perl(Exporter)
perl(Fatal)
perl(File::Copy)
perl(File::Find)
perl(File::ShareDir)
perl(File::ShareDir)
perl(File::Spec)
perl(File::Temp)
perl(HTML::FormFu)
perl(HTML::FormFu::Attribute)
perl(HTML::FormFu::Constants)
perl(HTML::FormFu::Constraint)
perl(HTML::FormFu::Deploy)
perl(HTML::FormFu::Element::_Field)
perl(HTML::FormFu::Element::_Group)
perl(HTML::FormFu::Exception)
perl(HTML::FormFu::Exception::Constraint)
perl(HTML::FormFu::Exception::Inflator)
perl(HTML::FormFu::Exception::Transformer)
perl(HTML::FormFu::Exception::Validator)
perl(HTML::FormFu::FakeQuery)
perl(HTML::FormFu::Filter)
perl(HTML::FormFu::Inflator)
perl(HTML::FormFu::Literal)
perl(HTML::FormFu::Localize)
perl(HTML::FormFu::ObjectUtil)
perl(HTML::FormFu::UploadParam)
perl(HTML::FormFu::Util)
perl(HTML::Scrubber)
perl(HTML::Scrubber)
perl(HTML::TokeParser::Simple)
perl(HTML::TokeParser::Simple) >= 3.14
perl(HTTP::Headers)
perl(HTTP::Headers) >= 1.64
perl(List::MoreUtils)
perl(List::MoreUtils)
perl(List::Util)
perl(Locale::Maketext::Simple)
perl(Module::Pluggable)
perl(Module::Pluggable)
perl(Readonly)
perl(Readonly)
perl(Regexp::Assemble)
perl(Regexp::Common)
perl(Regexp::Common)
perl(Regexp::Copy)
perl(Regexp::Copy)
perl(Scalar::Util)
perl(Storable)
perl(Task::Weaken)
perl(Template)
perl(YAML::Syck) >= 1.05
perl(base)
perl(defaults)
perl(model_config)
perl(overload)
perl(strict)
perl(utf8)
perl(warnings)
rpmlib(CompressedFileNames) <= 3.0.4-1
rpmlib(PayloadFilesHavePrefix) <= 4.0-1
rpmlib(VersionedDependencies) <= 3.0.3-1

Otherwise things are OK, come back with a revised spec and I'll gladly approve the package

Comment 2 manuel wolfshant 2008-11-29 13:42:31 UTC
While you are at it, could you also please recheck if the explicit BR for perl is really needed ?

Comment 3 Iain Arnell 2008-11-29 15:14:57 UTC
I've removed the unversioned duplicate requires and the br for perl. Obscure requires (Task::Weaken, Crypt:CBC, etc.) and versioned required remain.

New SRPM URL:
http://iarnell.fedorapeople.org/perl-HTML-FormFu-0.03005-2.fc11.src.rpm

Comment 4 manuel wolfshant 2008-11-29 17:27:24 UTC
Are you sure that all those versioned requires are needed ? After a quick glance in the source I've only found a reference to DateTime > 0.38.


Package Review
==============

Key:
 - = N/A
 x = Check
 ! = Problem
 ? = Not evaluated

=== REQUIRED ITEMS ===
 [x] Package is named according to the Package Naming Guidelines.
 [x] Spec file name must match the base package %{name}, in the format %{name}.spec.
 [x] Package meets the Packaging Guidelines including the Perl specific items
 [x] Package successfully compiles and builds into binary rpms on at least one supported architecture.
     Tested on: devel/x86_64
 [x] Rpmlint output:
source RPM: empty
binary RPM:empty
 [x] Package is not relocatable.
 [x] Buildroot is correct (%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n))
 [x] Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines.
 [x] License field in the package spec file matches the actual license.
     License type: GPL+ or Artistic
 [x] If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %doc.
 [x] Spec file is legible and written in American English.
 [x] Sources used to build the package matches the upstream source, as provided in the spec URL.
     SHA1SUM of package: f1bfd8c13b8c0949eefe3e99f6c8e93c41fb6970  HTML-FormFu-0.03005.tar.gz
 [x] Package is not known to require ExcludeArch
 [x] All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines.
 [-] The spec file handles locales properly.
 [-] ldconfig called in %post and %postun if required.
 [x] Package must own all directories that it creates.
 [x] Package requires other packages for directories it uses.
 [x] Package does not contain duplicates in %files.
 [x] Permissions on files are set properly.
 [x] Package has a %clean section, which contains rm -rf $RPM_BUILD_ROOT
 [x] Package consistently uses macros.
 [x] Package contains code, or permissable content.
 [-] Large documentation files are in a -doc subpackage, if required.
 [x] Package uses nothing in %doc for runtime.
 [-] Header files in -devel subpackage, if present.
 [-] Static libraries in -devel subpackage, if present.
 [-] Package requires pkgconfig, if .pc files are present.
 [-] Development .so files in -devel subpackage, if present.
 [-] Fully versioned dependency in subpackages, if present.
 [x] Package does not contain any libtool archives (.la).
 [-] Package contains a properly installed %{name}.desktop file if it is a GUI application.
 [x] Package does not own files or directories owned by other packages.

=== SUGGESTED ITEMS ===
 [x] Latest version is packaged.
 [x] Package does not include license text files separate from upstream.
 [-] Description and summary sections in the package spec file contains translations for supported Non-English languages, if available.
 [x] Reviewer should test that the package builds in mock.
     Tested on: rawhide/x86_64
 [?] Package should compile and build into binary rpms on all supported architectures.
     Tested on: noarch, perl module
 [?] Package functions as described.
 [-] Scriptlets must be sane, if used.
 [-] The placement of pkgconfig(.pc) files is correct.
 [-] File based requires are sane.
 [x] %check is present and the tests pass.

================
*** APPROVED *** but please examine again if those version restrictions are really needed, as they will block inclusion of this package in EPEL
================

Comment 5 Iain Arnell 2008-11-29 18:32:19 UTC
According to Makefile.PL, three are really required:

requires 'Config::Any' => '0.10'; # supports multi-doc config files
requires 'DateTime' => '0.38'; # required for string overloading
requires 'DateTime::Format::Builder' => '0.7901'; # fixes memory leaks

And one more that I'd missed and will add:

# this is the lowest version of Exporter I can identify that exports import()
# it's bundled with perl 5.83
# version 5.567 that ships with perl 5.82 is no good
requires 'Exporter' => '5.57';

Two don't really matter since they only exist in fedora since last week at compatible versions - no problem to remove these two:

requires 'Captcha::reCAPTCHA' => 0.92;
requires 'HTML::TokeParser::Simple' => '3.14';

HTTP::Headers seems to be an incredibly old version - no problem to remove this

requires 'HTTP::Headers' => '1.64';

And finally, YAML::Syck would also block inclusion in F-9. But it doesn't seem to be a direct requirement - more like strongly recommended for Config::Any. Will tentatively drop the version from this one since I don't see any strict need for 1.05.

requires 'YAML::Syck' => '1.05';


New spec and SRPM uploaded in case you want to re-review these changes:
http://iarnell.fedorapeople.org/perl-HTML-FormFu-0.03005-3.fc11.src.rpm

Comment 6 Chris Weyl 2008-11-29 21:18:25 UTC
(In reply to comment #5)

> And finally, YAML::Syck would also block inclusion in F-9. But it doesn't seem
> to be a direct requirement - more like strongly recommended for Config::Any.
> Will tentatively drop the version from this one since I don't see any strict
> need for 1.05.
> 
> requires 'YAML::Syck' => '1.05';

I just filed bug 473669 asking that it be updated in F-9, as 1.05 is already in F-10/rawhide.  If Steve doesn't respond within a day or two (and he's been non-responsive for months as near as I can tell, even to a private email), I'll build and push it.

Comment 7 Iain Arnell 2008-12-05 04:59:06 UTC
New Package CVS Request
=======================
Package Name: perl-HTML-FormFu
Short Description: HTML Form Creation, Rendering and Validation Framework
Owners: iarnell
Branches: F-9 F-10
InitialCC: perl-sig

Comment 8 Kevin Fenzi 2008-12-05 05:15:07 UTC
cvs done.

Comment 9 Fedora Update System 2008-12-05 15:37:17 UTC
perl-HTML-FormFu-0.03005-3.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/perl-HTML-FormFu-0.03005-3.fc10

Comment 10 Fedora Update System 2008-12-19 08:02:57 UTC
perl-HTML-FormFu-0.03007-1.fc9 has been submitted as an update for Fedora 9.
http://admin.fedoraproject.org/updates/perl-HTML-FormFu-0.03007-1.fc9

Comment 11 Fedora Update System 2009-01-07 09:21:35 UTC
perl-HTML-FormFu-0.03007-1.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.