Bug 428981 - Review Request: mod_line_edit - A DSO module for the apache web server
Summary: Review Request: mod_line_edit - A DSO module for the apache web server
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jason Tibbitts
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-01-16 16:05 UTC by rob
Modified: 2008-01-29 11:58 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-01-29 11:58:51 UTC
Type: ---
Embargoed:
j: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description rob 2008-01-16 16:05:55 UTC
Spec URL: http://rmyers.fedorapeople.org/mod_line_edit/mod_line_edit.spec
SRPM URL: http://rmyers.fedorapeople.org/mod_line_edit/mod_line_edit-1.0.0-2.el5.src.rpm

Description:

mod_line_edit is a general-purpose filter for text documents. It operates as a
simple on-the-fly line editor, applying search-and-replace rules defined in a
configuration or .htaccess file.

Unlike most of Webthing's filter modules, it is not markup-aware, so it is not 
an optimal choice for processing HTML or XML, though it may nevertheless be
used with caution (and may be far better than semi-markup-aware options such as
mod_layout).

For non-markup document types such as plain text, and non-markup Web documents
such as Javascript or Stylesheets, it is the best available option in the
absence of a filter that parses any relevant document structures.

mod_line_edit is written for performance and reliability, and should scale
without problems as document size grows. mod_line_edit is fully compatible with
Apache 2.0 and 2.2, and all operating systems and MPMs.

Note: this .spec was derived from the mandriva's apache-mod_line_edit-1.0.0-2mdv2008.0 package.

Comment 1 Jason Tibbitts 2008-01-19 18:41:15 UTC
Note that as far as I know, you can't link GPLv2 code with Apache.  You can
link GPLv3 code, however.  This package is GPLv2+, so I'd think it would be OK
but I'm not really an expert so I'll block FE-Legal.

Note to FE-Legal: there are plenty of GPL apache modules already in the
distro; in my checkout, five have License: tags of "GPL" and one
(mod_security) says "GPLv2".

I can, however, review the packaging.

* source files match upstream:
   5a71c8fc62cff97e9d8a0d20705daafc5200d990c33c7d1db156bd79db0d51da
   index.html
   9c6c33d401f545ebd3826df96a2ccaa07b25db0b99ba13e4d88b10a2d49f8b0b  
   mod_line_edit.c

The summary is a bit content-free; it describes all apache modules.  How about
  Symmary: A general-puropse filter for text documents
which is still a bit vague but I can't think of anything better at the moment.
(Additionally, there's no point in including the name of the package in the
summary.)

In your %install and %clean sections, there's no point in the tests checking
that the buildroot is not /; you set the buildroot to something that's not /
earlier in the spec.

So just a couple of minor tweaks and I would approve this package, assuming
that Legal signs off on it and indicates what the License: tag should be.

* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
X summary is somewhat content-free
* description is OK.
* dist tag is present.
* build root is OK.
? unsure about the license
* license text not included upstream (though the license header is extracted
  from the source and included as %doc)
* latest version is being packaged.
* BuildRequires are proper.
* compiler flags are appropriate.
X %clean is present, but not in the usual format.
* package builds in mock (rawhide, x86_64).
* package installs properly
* debuginfo package looks complete.
* rpmlint is silent.
* final provides and requires are sane:
   config(mod_line_edit) = 1.0.0-2.fc9
   mod_line_edit.so()(64bit)
   mod_line_edit = 1.0.0-2.fc9
  =
   config(mod_line_edit) = 1.0.0-2.fc9
   httpd
   httpd-mmn = 20051115

* %check is not present; no test suite upstream.  I have not tested this
   package.
* no shared libraries are added to the regular linker search paths.
* 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 -doc subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* no headers.
* no pkgconfig files.
* no static libraries.
* no libtool .la files.


Comment 2 Tom "spot" Callaway 2008-01-19 19:22:12 UTC
GPLv2+ is fine for this. We should look into the GPlv2 only item, though.
Lifting FE-Legal.

Comment 3 rob 2008-01-22 16:41:10 UTC
tibbs-

thanks for the quick and thorough package review.  i believe i have addressed
your concerns in the updated packages.

Spec Diff: http://rmyers.fedorapeople.org/mod_line_edit/spec.diff

Updated URLS:

Spec URL: http://rmyers.fedorapeople.org/mod_line_edit/mod_line_edit.spec
SRPM URL:
http://rmyers.fedorapeople.org/mod_line_edit/mod_line_edit-1.0.0-3.el5.src.rpm


Comment 4 Jason Tibbitts 2008-01-22 22:43:53 UTC
Looks good to me, and legal has signed off, so...

APPROVED

Comment 5 rob 2008-01-23 00:04:47 UTC
New Package CVS Request
=======================
Package Name: mod_line_edit
Short Description: A general-purpose filter for text documents
Owners: rmyers
Branches: EL-5 devel


Comment 6 Kevin Fenzi 2008-01-23 18:39:47 UTC
cvs done.


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