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@xxxxxxxxx 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. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review