Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 200600 - Review Request: phpPgAdmin - web based PostgreSQL administration
Summary: Review Request: phpPgAdmin - web based PostgreSQL administration
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Ruben Kerkhof
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-07-28 22:11 UTC by Devrim GUNDUZ
Modified: 2007-11-30 22:11 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-12-21 22:34:34 UTC
Type: ---
Embargoed:
petersen: fedora-cvs+


Attachments (Terms of Use)

Description Devrim GUNDUZ 2006-07-28 22:11:51 UTC
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-29 00:03:58 UTC
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-29 00:17:16 UTC
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-29 00:41:44 UTC
I am 100% positive that there is no need for this package to wait on any guidelines.

Comment 4 Chris Weyl 2006-07-29 00:48:58 UTC
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 21:41:40 UTC
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 21:52:48 UTC
(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 22:27:09 UTC
#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 23:23:56 UTC
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 20:01:10 UTC
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 20:54:24 UTC
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 06:18:39 UTC
What is left for the approval of this package?

Regards, Devrim

Comment 13 Ruben Kerkhof 2006-12-21 21:12:57 UTC
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 21:25:58 UTC
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 21:38:22 UTC
Two last issues:

- Replace usr/share with %{datadir}
- Add /sbin/service to Requires(post)

Comment 16 Devrim GUNDUZ 2006-12-21 21:59:31 UTC
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 22:10:44 UTC
Looks good, moving to FE-ACCEPT

Comment 18 Devrim GUNDUZ 2007-03-26 19:57:33 UTC
Package Change Request
======================
Package Name: phpPgAdmin
New Branches: EL-4 EL-5

Comment 19 Jens Petersen 2007-03-27 12:34:03 UTC
done


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