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