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

Bug 188351

Summary: Review Request: gphpedit - GNOME2 PHP editor
Product: [Fedora] Fedora Reporter: Tim Jackson <rpm>
Component: Package ReviewAssignee: Christoph Wickert <christoph.wickert>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: wtogami
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2006-04-17 20:37:28 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Bug Depends On:    
Bug Blocks: 163779    

Description Tim Jackson 2006-04-08 09:20:03 UTC
Spec Name or Url:
SRPM Name or Url:
gPHPEdit is a GNOME2 editor dedicated to editing PHP files and other 
supporting files like HTML/CSS.

Comment 1 Christoph Wickert 2006-04-13 22:45:35 UTC
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.

Comment 2 Tim Jackson 2006-04-13 23:17:04 UTC
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

Spec in same place as before.

Comment 3 Christoph Wickert 2006-04-15 00:59:11 UTC
f8cf9167b19c07f202d2d5e31431648a  gphpedit-0.9.80-2.src.rpm

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


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

Comment 4 Tim Jackson 2006-04-15 02:04:23 UTC
Thanks very much for the review Christoph.
Imported into CVS; the first build failed on x86_64
( 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.

Comment 5 Warren Togami 2006-04-17 15:32:07 UTC
Precomplied binaries are unacceptable.  It must be built from scratch, and maybe
in its own package that this depends on.

Comment 6 Tim Jackson 2006-04-17 20:15:26 UTC
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

Comment 7 Tim Jackson 2006-04-17 20:37:28 UTC
Looking healthier now; the current version now builds on all archs.

Comment 8 Tim Jackson 2006-04-18 21:17:00 UTC
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.