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

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.

I am 100% positive that there is no need for this package to wait on any guidelines.

My bad -- for some reason my little brain keeps on broadening the scope of the
php guidelines...

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.

(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)

#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?

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

Hello. New spec and SRPM:
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-3.src.rpm

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.

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.

What is left for the approval of this package?

Regards, Devrim

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.

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

Two last issues:

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

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

Looks good, moving to FE-ACCEPT

Package Change Request
======================
Package Name: phpPgAdmin
New Branches: EL-4 EL-5

done