Bug 235790 (perl-CGI-Prototype)

Summary: Review Request: perl-CGI-Prototype - Create a CGI application by subclassing
Product: [Fedora] Fedora Reporter: Chris Weyl <cweyl>
Component: Package ReviewAssignee: Bernard Johnson <bjohnson>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideFlags: bjohnson: fedora-review+
jwboyer: fedora-cvs+
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
URL: http://search.cpan.org/dist/CGI-Prototype/
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2007-05-04 15:14:19 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On: 235780    
Bug Blocks:    

Description Chris Weyl 2007-04-10 06:01:52 UTC
SRPM URL: http://home.comcast.net/~ckweyl/perl-CGI-Prototype-0.9053-1.fc6.src.rpm
SPEC URL: http://home.comcast.net/~ckweyl/perl-CGI-Prototype.spec

Description:
The core of every CGI application seems to be roughly the same:

*   Analyze the incoming parameters, cookies, and URLs to determine the state
of the application (let's call this "dispatch").
* Based on the current state, analyze the incoming parameters to respond to
any form submitted ("respond").
*   From there, decide what response page should be generated, and produce it
("render").

CGI::Prototype creates a "Class::Prototyped" engine for doing all this, with
the right amount of callback hooks to customize the process.  Because I'm
biased toward Template Toolkit for rendering HTML, I've also integrated that
as my rendering engine of choice. And, being a fan of clean MVC designs, the
classes become the controllers, and the templates become the views, with clean
separation of responsibilities, and "CGI::Prototype" a sort of "archetypal"
controller.

Comment 1 Bernard Johnson 2007-04-30 22:03:10 UTC
> # note the "skipped: CGI::Prototype::Mecha not found" is expected; this module
> # is a runtime requirement of that module, resulting in a
> # plugin-before-the-base-module sorta deal.

Are your intentions to submit this package?

Also, I noticed this:
Provides: perl(CGI::Prototype) = 0.9053 perl(CGI::Prototype::Hidden)
perl(My::App) perl(My::App::thanks) perl(My::App::welcome)

I haven't checked to see what perl(MY::App), perl(My::App::thanks), and
perl(My::App:welcome) are yet, but they seem a bit dubious.  Although probably
harmless as a Provide, you might want to make sure that's right.


Comment 2 Chris Weyl 2007-04-30 22:35:09 UTC
(In reply to comment #1)
> Are your intentions to submit this package?

Not at the moment.  Even if it were in, I'd be unable to list it as a BR for
this package without creating a circular dependency, as it requires this package
to function.

> Also, I noticed this:
> Provides: perl(CGI::Prototype) = 0.9053 perl(CGI::Prototype::Hidden)
> perl(My::App) perl(My::App::thanks) perl(My::App::welcome)
> 
> I haven't checked to see what perl(MY::App), perl(My::App::thanks), and
> perl(My::App:welcome) are yet, but they seem a bit dubious.  Although probably
> harmless as a Provide, you might want to make sure that's right.

Good catch -- for whatever reason it looks like the autoprov script was picking
that up.

SRPM URL: http://home.comcast.net/~ckweyl/perl-CGI-Prototype-0.9053-2.fc6.src.rpm
SPEC URL: http://home.comcast.net/~ckweyl/perl-CGI-Prototype.spec


Comment 3 Bernard Johnson 2007-04-30 23:07:30 UTC
> - disable autoprov -- with this small of a package it's easier than filtering

Well, maybe.  But I'd argue that filtering is more reliable.  If you update the
package at a later time and don't notice new functionality that would have a
provide, the filtering will automatically pick it up.  Your method won't.

> - include full test suite in %%doc

What's the rationale here? Is this needed?

Comment 4 Chris Weyl 2007-05-01 02:48:20 UTC
(In reply to comment #3)
> > - disable autoprov -- with this small of a package it's easier than filtering
> 
> Well, maybe.  But I'd argue that filtering is more reliable.  If you update the
> package at a later time and don't notice new functionality that would have a
> provide, the filtering will automatically pick it up.  Your method won't.

No problem; switched to a filtering approach.
 
> > - include full test suite in %%doc
> 
> What's the rationale here? Is this needed?

Well, technically nothing in %doc is needed :)

I've been realizing more and more lately that test suites can also make good
documentation, so I've started including them verbatim in %doc.  Some of them --
like Moose -- even go so far as to say "the best documentation for [this
feature] is still in the test suite."  

Even beyond that, it's certainly possible that someone will want to test ths
package as installed -- having the test suite around enables such activity.

SRPM URL: http://home.comcast.net/~ckweyl/perl-CGI-Prototype-0.9053-3.fc6.src.rpm
SPEC URL: http://home.comcast.net/~ckweyl/perl-CGI-Prototype.spec




Comment 5 Bernard Johnson 2007-05-01 03:03:06 UTC
(In reply to comment #4)
> Well, technically nothing in %doc is needed :)

All right, let's not be too pedantic :)

> I've been realizing more and more lately that test suites can also make good
> documentation, so I've started including them verbatim in %doc.  Some of them --
> like Moose -- even go so far as to say "the best documentation for [this
> feature] is still in the test suite."  
 
So what I read is "this test suite serves as a good addition to the
documentation" - which is good enough for me.  I just haven't seen anyone else
packaging the test scripts, except as separate packages like mysql-test.

> Even beyond that, it's certainly possible that someone will want to test ths
> package as installed -- having the test suite around enables such activity.

Well, in theory, if it passed in the buildsystem, it should be good, but....



Comment 6 Chris Weyl 2007-05-01 03:08:58 UTC
(In reply to comment #5)
> (In reply to comment #4)
> > Well, technically nothing in %doc is needed :)
> 
> All right, let's not be too pedantic :)

Who, me?  O:-)

> So what I read is "this test suite serves as a good addition to the
> documentation" - which is good enough for me.  I just haven't seen anyone else
> packaging the test scripts, except as separate packages like mysql-test.

Yah.  I think we just never really saw it as anything more than a part of the
build process before.

> > Even beyond that, it's certainly possible that someone will want to test ths
> > package as installed -- having the test suite around enables such activity.
> 
> Well, in theory, if it passed in the buildsystem, it should be good, but....

That's another thing, too.  If a package's test suite requires network access,
we have to disable it by default, as it'd bomb out in mock.  WWW::Myspace or
most of the POE::Component::* modules are good examples of this: good, solid
test suites I have to disable large parts of by default.

Comment 7 Bernard Johnson 2007-05-01 05:55:08 UTC
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.
 [x] Package successfully compiles and builds into binary rpms on at least one
supported architecture.
     Tested on: FC-6 / i386
 [x] Rpmlint output: None
 [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
 [-] 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.
 [-] 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.
     MD5SUM this package    : 0d09d6fcf9616789ca2efbefbc7ed401
     MD5SUM upstream package: 0d09d6fcf9616789ca2efbefbc7ed401
 [x] Package is not known to require ExcludeArch, OR:
     Arches excluded:
     Why:
 [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.
 [-] 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 %{buildroot} (or
$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: FC-6 / i386
 [-] Package should compile and build into binary rpms on all supported
architectures.
     Tested on:
 [?] Package functions as described.
 [-] Scriptlets must be sane, if used.
 [-] The placement of pkgconfig(.pc) files are correct.
 [-] File based requires are sane.


=== Issues ===
1.

=== Final Notes ===
1.


================
*** APPROVED ***
================

Comment 8 Chris Weyl 2007-05-01 15:06:02 UTC
New Package CVS Request
=======================
Package Name: perl-CGI-Prototype
Short Description: Create a CGI application by subclassing
Owners: cweyl.edu
Branches: FC-5, FC-6, devel
InitialCC: fedora-perl-devel-list

Comment 9 Chris Weyl 2007-05-04 15:14:19 UTC
Imported and building.  Thanks for the review!