Bug 200600 - Review Request: phpPgAdmin - web based PostgreSQL administration
Review Request: phpPgAdmin - web based PostgreSQL administration
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Ruben Kerkhof
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-07-28 18:11 EDT by Devrim GUNDUZ
Modified: 2007-11-30 17:11 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-12-21 17:34:34 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
petersen: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Devrim GUNDUZ 2006-07-28 18:11:51 EDT
Spec URL: http://developer.postgresql.org/~devrim/rpms/other/phpPgAdmin/phpPgAdmin.spec
SRPM URL: http://developer.postgresql.org/~devrim/rpms/other/phpPgAdmin/phpPgAdmin-4.0.1-2.src.rpm
Description: phpPgAdmin is a fully functional web-based administration utility for
a PostgreSQL database server. It handles all the basic functionality
as well as some advanced features such as triggers, views and
functions (stored procedures). It also has Slony-I support.
Comment 1 Chris Weyl 2006-07-28 20:03:58 EDT
Devrim, I suspect you should block FE-GUIDELINES as well, given that the php
guidelines haven't been formally announced yet...  (but should be shortly.)
Comment 2 Toshio Kuratomi 2006-07-28 20:17:16 EDT
I am 98% positive that the php guidelines are only needed for extensions to php.
 Web applications need to follow the web application sections of the guidelines
but otherwise are fine.
Comment 3 Jason Tibbitts 2006-07-28 20:41:44 EDT
I am 100% positive that there is no need for this package to wait on any guidelines.
Comment 4 Chris Weyl 2006-07-28 20:48:58 EDT
My bad -- for some reason my little brain keeps on broadening the scope of the
php guidelines...
Comment 5 Paul F. Johnson 2006-08-01 17:41:40 EDT
Some comments

Release:	2

Needs %{?dist} on the end

Requires:	webserver

Never heard of it! should this be httpd?

%attr(644,root,root)  %{_phppgadmindir}/lang/Makefile

A makefile? Sure about that?

%attr(755,root,root)  %{_phppgadmindir}/lang/convert.awk

I suspect you'll need R: awk

Just comments.
Comment 6 Chris Weyl 2006-08-01 17:52:48 EDT
(In reply to comment #5)
> Requires:	webserver
> 
> Never heard of it! should this be httpd?

httpd (and others) provide webserver.  (e.g. yum whatprovides webserver)
Comment 7 Paul F. Johnson 2006-08-01 18:27:09 EDT
#5, true. However, given the spec file later specifies httpd, would this not be
better off as requiring httpd so that no other webserver providing application
is accepted?
Comment 8 Devrim GUNDUZ 2006-08-01 19:23:56 EDT
Hi,

(In reply to comment #5)
> Release:	2
> 
> Needs %{?dist} on the end

Thanks! Fixed.
 
> Requires:	webserver
> 
> Never heard of it! should this be httpd?

Skipping this per comments above.

> %attr(644,root,root)  %{_phppgadmindir}/lang/Makefile
> 
> A makefile? Sure about that?

Yeah. It is the Makefile for language utility that ships with phpPgAdmin.
(thought a bit more before submitting this comment...) It **may** be enough to
ship lang/recoded/ dir; however I'm inclined to ship source lang files and make
people correct and compile their languages easily. Let's leave that like this.
 
> %attr(755,root,root)  %{_phppgadmindir}/lang/convert.awk
> 
> I suspect you'll need R: awk

Thanks, added.

Regards, Devrim
Comment 10 Gwyn Ciesla 2006-11-09 15:01:10 EST
As a non-sponsored would-be contributor, I'd like to contribute the following:

A: I've downloaded the most recent srpm and spec and run over the MUST and
SHOULD lists from ReviewGuidelines, and it checks out AFAICT.  Also, as a
longtime user of this app on Fedora installing from tarballs, I can say that it
works as expected.

B: Is there a reason to Require either webserver OR httpd?  Does not php require
httpd which would then satisfy webserver?

Obviously I can't complete the formal review, just thought I'd put in my $0.02 US.
Comment 11 Gwyn Ciesla 2006-11-09 15:54:24 EST
Incidentally, as I am new to this process, if I have overlooked anything, I'd be
very keen on knowing what.

(In reply to comment #10)
> As a non-sponsored would-be contributor, I'd like to contribute the following:
> 
> A: I've downloaded the most recent srpm and spec and run over the MUST and
> SHOULD lists from ReviewGuidelines, and it checks out AFAICT.  Also, as a
> longtime user of this app on Fedora installing from tarballs, I can say that it
> works as expected.
> 
> B: Is there a reason to Require either webserver OR httpd?  Does not php require
> httpd which would then satisfy webserver?
> 
> Obviously I can't complete the formal review, just thought I'd put in my $0.02 US.

Comment 12 Devrim GUNDUZ 2006-12-06 01:18:39 EST
What is left for the approval of this package?

Regards, Devrim
Comment 13 Ruben Kerkhof 2006-12-21 16:12:57 EST
Hi Devrim,

A few remarks:

- Please don't include the name of the package in the Summary
- Please don't use the Vendor tag
- Web applications should not put their content in /var/www (see Guidelines/Web Applications in the 
Wiki
- It fails to build in mock, since /etc/httpd/conf.d does not exist over there. I think it's safe to skip the 
check for that directory, and install -d your config file
- httpd reload in %post fails if you don't have httpd installed, but another webserver, but I'm not sure 
how to handle that.
Comment 14 Devrim GUNDUZ 2006-12-21 16:25:58 EST
Hi,

(In reply to comment #13)
> A few remarks:
> 
> - Please don't include the name of the package in the Summary
> - Please don't use the Vendor tag
> - Web applications should not put their content in /var/www (see
Guidelines/Web Applications in the 
> Wiki
> - It fails to build in mock, since /etc/httpd/conf.d does not exist over
there. I think it's safe to skip the 
> check for that directory, and install -d your config file

Both are fixed.

> - httpd reload in %post fails if you don't have httpd installed, but another
webserver, but I'm not sure 
> how to handle that.

This package now requires httpd directly.

Spec URL:
http://developer.postgresql.org/~devrim/rpms/other/phpPgAdmin/phpPgAdmin.spec
SRPM URL:
http://developer.postgresql.org/~devrim/rpms/other/phpPgAdmin/phpPgAdmin-4.0.1-4.src.rpm

Thanks, Devrim
Comment 15 Ruben Kerkhof 2006-12-21 16:38:22 EST
Two last issues:

- Replace usr/share with %{datadir}
- Add /sbin/service to Requires(post)
Comment 16 Devrim GUNDUZ 2006-12-21 16:59:31 EST
Hi,

(In reply to comment #15)
> Two last issues:
> 
> - Replace usr/share with %{datadir}
> - Add /sbin/service to Requires(post)

Both are done:

Spec URL:
http://developer.postgresql.org/~devrim/rpms/other/phpPgAdmin/phpPgAdmin.spec
SRPM URL:
http://developer.postgresql.org/~devrim/rpms/other/phpPgAdmin/phpPgAdmin-4.0.1-5.src.rpm

Regards, Devrim
Comment 17 Ruben Kerkhof 2006-12-21 17:10:44 EST
Looks good, moving to FE-ACCEPT
Comment 18 Devrim GUNDUZ 2007-03-26 15:57:33 EDT
Package Change Request
======================
Package Name: phpPgAdmin
New Branches: EL-4 EL-5
Comment 19 Jens Petersen 2007-03-27 08:34:03 EDT
done

Note You need to log in before you can comment on or make changes to this bug.