[Bug 200630] Review Request: postgresql_autodoc - PostgreSQL AutoDoc Utility

[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_autodoc - PostgreSQL AutoDoc Utility


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


toshio at tiki-lounge.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|bugzilla-sink at leemhuis.info |toshio at tiki-lounge.com
OtherBugsDependingO|163776                      |163778
              nThis|                            |




------- Additional Comments From toshio at tiki-lounge.com  2006-08-19 15:54 EST -------
MD5Sums:
ef45084bb065b11def33ef7885ee694b  postgresql_autodoc-1.25-3.src.rpm
f61071a23f6b34f948bbf799de91e8e4  postgresql_autodoc-1.25.tar.gz
72f9c48a19b9a0d2999f1274e0e4398d  postgresql_autodoc.spec

Blockers:
* Package installs to %{_datadir}/pgsql but it neither owns the directory nor
  depends on anything which requires it.  This may be a bug in the postgresql
  packaging.  Care to ask Tom Lane if the postgresql package rather than
  postgresql-server should own %{_datadir}/pgsql?  I don't know the purpose of
  the directories well enough to judge.
* Package does not own %{_datadir}/pgsql/postgresql_autodoc

Cosmetic:
* Perl packages typically have virtual provides detailing what perl
  dependencies they are providing.  The prefered way to Require perl modules
  is through this virtual provide method.  So instead of
    BuildRequires: perl-HTML-Template
    BuildRequires: perl-Pg-DBD
  you want to have:
    BuildRequire: perl(DBD::Pg)
    BuildRequires: perl(HTML::Template)
* Using %{?dist} in the release makes upgrades across Fedora Core releases
  work more or less seamlessly.  Consider adding %{?dist} to the end of your
  Release: tag.
* The package does not come with a detached license file.  You should ask
  upstream to include one next time they release a tarball.  (Since the license
  is included as part of the source code and this is a script so it is in the
  installed package, this is not a blocker.  But it is convenient for end-users
  to have this.)
* When manually installing files in the spec file you should try to preserve
  the file timestamps.  This can be done with cp -p in your

Questions:
* If I'm reading the source correctly, this package will only work with
  postgresql, not other db's that use the perl DBI interface.  But the
  Requires picked up by rpm do not include perl(DBD::Pg).  Should there
  be a Requires: perl(DBD:Pg) in the spec?
* Running the program just errors for me.  Any clues?
    $ postgresql_autodoc  -d orchard --password='XXXXX'
    Can't call method "finish" on an undefined value at
/usr/bin/postgresql_autodoc line 1203.
* It appears that the only method for providing a password to use when
  connecting to the database is via the commandline.  This is insecure as it
  allows another user to see the password with something as simple as the
  "ps" command.  It would be a very good idea to ask upstream for other methods
  of sending the password: prompt, config file, etc.

Good:
* Source matches upstream
* Package follows naming guidelines. (The _ comes from the upstream project).
* spec file name matches the package name.
* The license is original BSD and matches in SOURCE and spec file.
* The spec file is readable.
* No locales so %find_lang is not present.
* Not a library package.
* Not relocatable.
* Does not contain duplicate files.
* Package has a proper %clean section.
* Code, not content.
* Macros are used consistently.
* No %doc affects the application runtime.
* Not a GUI application.
* Does not own directories or files owned by someone else.
* No scriptlets.
* Builds in mock.
* Permissions are okay.

Notes:
* rpmlint outputs:
    $ rpmlint postgresql_autodoc-1.25-3.*
    W: postgresql_autodoc no-documentation
  Which is ignorable because the package doesn't provide any documentation
  at this time.


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