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: pgFouine - PostgreSQL log analyzer https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=202901 toshio@xxxxxxxxxxxxxxx changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED AssignedTo|nobody@xxxxxxxxxxxxxxxxx |toshio@xxxxxxxxxxxxxxx OtherBugsDependingO|163776 |163778 nThis| | ------- Additional Comments From toshio@xxxxxxxxxxxxxxx 2006-08-18 01:48 EST ------- MD5Sums 249021f97fc8f90836205db8d5c2cd5a pgfouine-0.7-3.src.rpm c6b09d495fd11e0b8e9b4b86e4252449 pgfouine-0.7-include_path.patch 536a23564b21eb28d98485a3746b90a5 pgfouine-0.7.tar.gz 13783dd119055e07a1f4bb5822b5e768 pgfouine.spec Blockers: * Source does not match upstream tarball. It looks like upstream has taken some changes from you and you have repackaged the 0.7 release with these changes included. This is not okay. Instead you can either: 1) Use the vanilla 0.7 tarball with any necessary patches applied afterwards. 2) Use a snapshot from upstream's SCM and in a comment show how to recreate the snapshot package. Since a diff of the files doesn't show any changes that will affect building or what is installed on the system, I would suggest using upstream's 0.7 tarball in this case. * Macros are used everywhere except in the patch file. So if %{_datadir} is ever redefined, the include_path will still be set to /usr/share/.... I would suggest using a patch file that does something like: + ini_set('include_path', '@INCLUDEPATH@'); + And then in the spec using: sed -i 's!@INCLUDEPATH@!%{_datadir}!' pgfouine_vacuum.php sed -i 's!@INCLUDEPATH@!%{_datadir}!' pgfouine.php If the upstream package had a build script it would do something like that to allow the paths to be redefined. * INSTALL is not needed in the package as it doesn't give any information that would be relevant to someone who installed via the rpm. ChangeLog could go in but that depends on how useful you think it will be to users of the package. * Why are the tests installed into %{_datadir}/pgfouine? Are they necessary for the package to run? If not, they should be installed to %doc (if they are useful for the user to know how to run the programs) or left out. Cosmetic: * There's no need to check that the buildroot is not "/" before rm'ing it because you are already setting the buildroot in the spec file. So: [ "%{buildroot}" != "/" ] && rm -rf %{buildroot} can be reduced to: rm -rf %{buildroot} * I favor more verbose Changelog entries. Since the previous reviewing occurred on IRC rather than bugzilla, it would be especially nice (When in bugzilla, you can reference the bugzilla number; when on IRC, things can get lost.) Good and Already Fixed: * Name conforms to the naming guidelines. * Spec file name matches the package name. * Package is licensed under the GPL and this is reflected in the license tag. * License is included as the COPYING file. * Spec file is written in English and is legible. * Builds to noarch on x86_64. * No language files, no need for %find_lang. * No shared libraries. * Not relocatable. * Owns all directories that are created. * No duplicate files listed in %files. * permissions are set correctly on files. * %clean section that removes the buildroot is present. * Code not content. * Nothing in %doc affects the program's operation. * Not a library package so no headers, static or dynamic libs, pkgconfig files, or .la files. * Not a GUI application so no .desktop needed. * Does not own files or directories owned by another package. * No scriptlets included or needed. * No subpackages * Removed AutoReq: no and Requires: php. This change let rpm's dependency resolver pick up the dependence on /usr/bin/php on its own. * Changed patch to remove the "." path from being included in the scripts. This was a security hole. (Invoke the script from a world writable directory and someone could inject a trojan php file into the script.) * Packager, Vendor, Copyright, and PreReq tags are not used as per Packaging/Guidelines. * BuildRoot in prefered form. * Builds in mock. * No necessary or optional buildrequires were left out. * rpmlint returns no issues for the package. -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug, or are watching the QA contact. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review