[Bug 202901] Review Request: pgFouine - PostgreSQL log analyzer

bugzilla at redhat.com bugzilla at redhat.com
Fri Sep 8 13:58:51 UTC 2006


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





------- Additional Comments From guillaume.smet at gmail.com  2006-09-08 09:58 EST -------
Here is the details of what we fixed and how we fixed it in the spec/src.rpm
Devrim just posted (0.7-4):

> * Source does not match upstream tarball.

It was an error of mine when I sent the first tarball to Devrim. It's now fixed
- the src.rpm contains a vanilla 0.7 release.

> * Macros are used everywhere except in the patch file.

We use a sed command in the %prep as you suggest it.

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

INSTALL removed, ChangeLog added - it can be useful to check if a problem has
been solved in the parser.

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

They are useless for the user. We removed them.

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

Fixed.

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

OK. We will take that into account.

Regards,

--
Guillaume

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




More information about the Fedora-package-review mailing list