[Bug 166203] Review Request: perl-Maypole : MVC web application framework

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: perl-Maypole : MVC web application framework


https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=166203


chris at chrisgrau.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|gdk at redhat.com              |chris at chrisgrau.com
OtherBugsDependingO|163776                      |163778
              nThis|                            |




------- Additional Comments From chris at chrisgrau.com  2005-10-06 14:06 EST -------
GOOD:

- rpmlint clean
- package name okay
- spec file name okay
- compatible license, matches upstream
- spec file is legible and am. english
- sources match upstream
  5d908a447d4e6f6364403aa56b1ab211  Maypole-2.10.tar.gz
  486cc148ab9b9aab52cc4035994f1c43  Maypole-Plugin-Session-0.2.tar.gz
- package builds in mock (FC-4 x86)
- no locales
- no shared libs
- not relocatable
- owns all created directories
- no duplicate files
- file permissions okay
- %clean okay
- consistent use of macros
- code, not content
- no -doc or -devel, %doc okay

Questions/Comments/Needswork:

Missing requires on perl(Apache2::Request) (see below).

You'll get more test coverage with,
  BR: perl(Test::Pod) >= 1.14
  BR: perl(Test::Pod::Coverage) >= 1.04
  (version numbers probably not be needed)

It's probably too much to hope that t/apache_mvc can be run successfully, since
it uses Apache::Request, which is now Apache2::Request.  An rt.cpan.org bug
report (#13888) seems to indicate that this will be fixed in an upcoming release.

The test suite for Maypole-Plugin-Session isn't run at all.

The Maypole Makefile.PL lists the following prerequisites, which are not listed
as BRs.
  - Class::DBI::Loader  => '0.02',
  - Class::DBI          => 0.96,
  - CGI::Untaint        => 0,
  - UNIVERSAL::moniker  => 0,
  - UNIVERSAL::require  => 0,
  - URI::QueryParam     => 0,
  - HTTP::Headers       => 1.59,
  - Template            => 0,
Since the package builds in mock, I'm assuming that other BRs pull these in
successfully.  Given the list of BRs, it's a safe bet that Class::DBI::Loader,
Class::DBI, and Template will always be pulled in, but I'd like to see them
listed explicitly anyway.

Why is Maypole-Plugin-Session being packaged inside perl-Maypole instead of
separately?  The distributions are separate on CPAN, with different authors. 
Will the versions always stay in sync?

perl-Maypole-2.10-Apache2only.patch:

- forces this to work on only FC-4+, which is fine; Mason did the same, I believe
- get_request() appears to need the requires on Apache::Request (or, in
mod_perl-2.0.x, Apache2::Request) to work; this patch would leave Apache::MVC broken
- no need to require Bundle::Apache2; it's only useful for CPAN shell installs

The patch attached to http://rt.cpan.org/NoAuth/Bug.html?id=13888 appears to
address the issues.

perl-Maypole-2.10-sessionfix.patch:

- is this necessary? (now I see why Maypole-Plugin-Session is being bundled)

On brief examination, Maypole-Plugin-Session's generate_unique_id() routine is
really a fancy pass-thru to Apache::Session's generate_unique_id() routine. 
Maypole::Session's genereate_unique_id() routine appears to simply be a copy of
the one found in Apache::Session.

I hope that was thorough enough.


-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.