[Bug 246747] Review Request: postgresql-ip4r - IPv4 and IPv4 range index types for PostgreSQL

[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: postgresql-ip4r - IPv4 and IPv4 range index types for PostgreSQL


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





------- Additional Comments From lindner at inuus.com  2007-07-09 09:23 EST -------
Here's my review, I'm not an official reviewer.  Anything marked with a ****
indicates potential problems that may need to be addressed.

MUST Items:
    * rpmlint - passes, no errors reported.
    * Package naming is good.  Uses same naming of upstream package with
postgresql prefix %{parent}-%{name}.  Version is all numeric.
    * Package name and spec file name match.
    * The package meets the Packaging Guidelines.

****  - License is BSD, which corresponds to pgfoundry page.  Actual source
distribution does not contain any Licensing attribution

      - No license in source, so no need to include in %doc
      - The spec file is written in American English, and is legible.
      - Upstream source matches source in the RPM
      - RPM builds on i386
      - BuildRequires appears sane.  The postgresql-devel will contain the
appropriate compiler requirements.
      - Locales are not a problem as they are not supported.

***** - ldconfig is called, however the .so files are not installed in the
system library search paths, so it appears that this step is redundant.

      - Relocation not supported, so no special checks for that.

***** - The package does not own the directory %{_datadir}/%{name}
      - MUST: A package must own all directories that it creates. If it does not
create a directory that it uses, then it should require a package which does
create that directory. The exception to this are directories listed explicitly
in the Filesystem Hierarchy Standard
(http://www.pathname.com/fhs/pub/fhs-2.3.html), as it is safe to assume that
those directories exist.

      - No duplicate files noted, permissions appear correct, defattr is
present. Correct %clean section is present.
      - Macro usage appears consistent.
      - RPM contains code, not content, no need for a -doc subpackage.
      - There are no dependencies on the %doc readme file.
      - No header files or static libraries, so no need for a -devel package.
      - rm -rf %buildroot is at the beginning of %install
      - All filenames are be valid UTF-8.

SHOULD Items:

***** - SHOULD: If the source package does not include license text(s) as a
separate file from upstream, the packager SHOULD query upstream to include it.

***** - SHOULD: The description and summary sections in the package spec file
should contain translations for supported Non-English languages, if available.
      - SHOULD: The reviewer should test that the package builds in mock.
        ---- I do not have mock installed..  Not tested.

      - SHOULD: The package should compile and build into binary rpms on all
supported architectures.
        ----- Only tested on i386

      - SHOULD: The reviewer should test that the package functions as
described. A package should not segfault instead of running, for example.
        ----- Works well on i386


***** - SHOULD: Usually, subpackages other than devel should require the base
package using a fully versioned dependency.

  It seems to me that this extension might now work with postgres 8.3, 8.4,
9.0??  The following Requires appears overly broad...

Requires:       postgresql-server >= 8.1

***** Consider adding the -p flag to the install commands for the sql and the
README file to preserve the original timestamps.


-- 
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.