Hi Tim, just a few comments:

- Source0 needs an absolute URL like http://...
- The %files section looks broken, the package doesn't own %{_datadir}/gphpedit/. 
- You are replacing a hardcoded path with another hardcoded one in the specfile.
Better use
sed -i s_/usr/local/share/pixmaps_%{_datadir}/pixmaps_ src/main.h
- Please add the categories "GNOME" and "Application" to gphpedit.desktop

The rest looks good to me. I'm going to do a review ASAP.

Thanks for the comments Christoph! All fixed, with the exception of Source0: the
upstream site doesn't have a proper URL to download from (it is obscured by a
download hit-tracker). I've already requested that the author adds a proper URL
(e.g. http://www.gphpedit.org/downloads/gphpedit-%{version}.tar.gz). 

New SRPM:
http://www.timj.co.uk/linux/srpms/gphpedit-0.9.80-2.src.rpm
Spec in same place as before.

REVIEW for
f8cf9167b19c07f202d2d5e31431648a  gphpedit-0.9.80-2.src.rpm

Good:
- rpmlint clean except for one zero-length error (see below)
- package and specfile naming according to Package Naming Guidelines
- package meets Packaging Guidlines
- license ok (GPL)
- license field in specfile machtes actual license
- license included in source and correctly installed in %doc
- specfile written in American English
- specfile legible. Actually that's what specs look like! :-) Clean and well
commented.
- source matches upstream
- package builds into binaries in Fedora Core 5 i386
- no locales to worry about
- no shared libs
- relocatable
- package owns all directories it creates
- package doesn't own files or dirs already owned by other packages
- no duplicates in %files listing
- permissions of files ok, %defattr correct
- %clean section present and correct
- macro usage consistent
- code, not content
- no large docs
- docs don't affect runtime
- no headers, pkgconfigs or static libs to worry about
- no libtool archives
- desktop file included and correctly installed

- package builds in mock for Core 5 i386
- program works fine, package has been testet for more than a week without problems
- changelog information correct and detailed 


APPROVED


You can now import the srpm into CVS, but there are four things you need to fix
before building:

- remove gtk2-devel from BuildRequires. It's a duplicate as it's already
required both by gtkhtml2-devel and libgnomeui-devel. The rest of the Requires
and BuildRequires are correct, none of the exceptions listed in the wiki.

- remove empty NEWS from %doc to fix this rpmlint error.
> rpmlint gphpedit-0.9.80-2.fc5.i386.rpm
> E: gphpedit zero-length /usr/share/doc/gphpedit-0.9.80/NEWS

- remove generic INSTALL from %doc (not needed)

- please add the URL for Source0 when you receive feedback from upstream

Thanks very much for the review Christoph.
Imported into CVS; the first build failed on x86_64
(http://buildsys.fedoraproject.org/build-status/job.psp?uid=7737) so I will look
into that. At a first glance it looks like something fishy upstream with
precompiled libraries bundled in the source tarball, so the spec may need
revisiting to rebuild those properly and appropriately for the arch.

Precomplied binaries are unacceptable.  It must be built from scratch, and maybe
in its own package that this depends on.

Yes, I know. Looking at it, it appears that the presence of the pre-existing
binaries may actually be accidental (perhaps an error during the release
process?); I will discuss with the author. A "make clean; make" recompiles the
objects in question cleanly, at least on x86; I will submit a new build shortly.
The presence of the library (gtkscintilla, "moleskine" version) bundled in the
package is intentional; the library is a modified version (compatible licensing
- GPL) and is effectively unmaintained upstream so the gPHPEdit author hasn't
got much choice but to bundle it. There were apparently extensive discussions
during the Debian packaging process for the same reason (I understand they hate
multiple copies of libraries with a vengeance) and it was agreed there that
bundling is appropriate in this case. (Note I am treating "bundling of
libraries" and "distribution of precompiled binaries" as totally separate issues
here)

Looking healthier now; the current version now builds on all archs.

Upstream author has confirmed that the inclusion of precompiled binaries was an
error in the release process; next upstream version won't have them and so the
"make clean" hack can be dropped.