Bug 457212

I would be happy to review this... look for a full review probibly tomorrow. 

Sorry for the delay...

OK - Package meets naming and packaging guidelines
OK - Spec file matches base package name.
OK - Spec has consistant macro usage.
OK - Meets Packaging Guidelines.
See below - License
See below- License field in spec matches
OK - License file included in package
OK - Spec in American English
OK - Spec is legible.
OK - Sources match upstream md5sum:
cdb23e367fc0d2f18c3717aabdc9ef21  gadmin-squid-0.1.0.tar.gz
cdb23e367fc0d2f18c3717aabdc9ef21  gadmin-squid-0.1.0.tar.gz.1
OK - BuildRequires correct
OK - Package has %defattr and permissions on files is good.
OK - Package has a correct %clean section.
OK - Package has correct buildroot
OK - Package is code or permissible content.
OK - Packages %doc files don't affect runtime.
OK - Package has rm -rf RPM_BUILD_ROOT at top of %install

OK - Package is a GUI app and has a .desktop file

OK - Package compiles and builds on at least one arch. 
OK - Package has no duplicate files in %files.
OK - Package doesn't own any directories other packages own.
OK - Package owns all the directories it creates.
See below - No rpmlint output.
OK - final provides and requires are sane.

SHOULD Items:

OK - Should build in mock.
OK - Should have dist tag
OK - Should package latest version

Issues:

1. Not sure the URL is right here. That site mentions that there is a new site
called www.gadmintools.org, which appears to be a dyndns redirect. In any case,
can you find a url that will point a user directly to the gadmin-squid page?

2. The license tag here seems to be 'GPLv3+' to me... ie, GPLv3 or later.

3. Might include the TODO and NEWS files as docs.

4. rpmlint says:

gadmin-squid.src: W: mixed-use-of-spaces-and-tabs (spaces: line 14, tab: line 1)

Can be ignored.

5. Why put the doc files in %{_docdir}/%{name}/ instead of just marking them
%doc and getting them under the standard place:
%{_docdir}/%{name}-%{version}-%{release}/ ?

I updated the spec file to reflect the issues raised by Kevin:

SPEC: http://www.kanarip.com/custom/SPECS/gadmin-squid.spec
SRPM: http://www.kanarip.com/custom/el5/SRPMS/gadmin-squid-0.1.0-0.3.el5.src.rpm

point 1: I don't know if the URL I updated to is a good one, but it is better than the previously used one, I'm sure.
point 2: although the license included is GPLv3, the src/* indicates indeed GPLv3+, so changed in the spec.
point 3: files added
point 4: although no problem, changed anyway :)
point 5: missed that one, upstream does it that way, changed in a patch and will contact upstream about it.

Humm... all the issues I saw appear to be solved, but you made an additional change I am unclear on:

Why add:

BuildRequires: autoconf
BuildRequires: automake

and change the configure line to use
%GNUconfigure

?

Kevin,

the BuildRequires and %GNUconfigure were neccesary to have it build for epel-5.
The source tarball contains an Makefile.in that borks the build for epel-5.
Including above statements ensures that Makefile.in is regenerated.

Looks like the docs are going to require a bit more tinkering... the latest package doesn't build here. I get: 

error: Installed (but unpackaged) file(s) found:
   /usr/share/doc/gadmin-squid/AUTHORS
   /usr/share/doc/gadmin-squid/COPYING
   /usr/share/doc/gadmin-squid/ChangeLog
   /usr/share/doc/gadmin-squid/README
    Installed (but unpackaged) file(s) found:
   /usr/share/doc/gadmin-squid/AUTHORS
   /usr/share/doc/gadmin-squid/COPYING
   /usr/share/doc/gadmin-squid/ChangeLog
   /usr/share/doc/gadmin-squid/README

If GNUconfigure is needed for EL-5, perhaps only use it in that branch, and/or use a conditional on the %dist tag.

I cannot reproduce your results Kevin.
Even on (fresh) bare metal never before used to build this package (or any other) I cannot reproduce this.
What was the command line you used?
As a side note I must say that EPEL-5 is not really a target, more a nice to have at this point.

I was building it here using 'mock'. 
See: http://fedoraproject.org/wiki/PackageMaintainers/MockTricks

for more info. 

I can also submit it as a scratch build in our buildsystem:
http://koji.fedoraproject.org/koji/taskinfo?taskID=758305

which seems to have finished fine. ;( 
I seem to have used the wrong spec or something is going on with my mock machine. ;( 

So, provided you conditionalize the GNUconfigure bits or only check in that version on the EL-5 branch, this package is APPROVED. 

I will go ahead and sponsor you, you can continue the process at: 
http://fedoraproject.org/wiki/PackageMaintainers/Join#Add_Package_to_CVS_and_Set_Owner

Kevin, how's this?

new Spec URL: http://www.kanarip.com/custom/SPECS/gadmin-squid.spec
new SRPM URL: http://www.kanarip.com/custom/f9/SRPMS/gadmin-squid-0.1.0-0.4.fc9.src.rpm

Changelog:
- Made autoconf and automake buildrequires conditional and remove
  %GNUconfigure from the .spec
- Added %{__aclocal} and %{__automake} to %prep stage in the .spec, because of outdated files inside the upstream tarball
- replaced another 'bare' make statement with %{__make} in the %install
  stage in the .spec

New Package CVS Request
=======================
Package Name: gadmin-squid
Short Description: graphical configuration tool for squid
Owners: baard
Branches: F-8 F-9 
InitialCC: [none]
Cvsextras Commits: yes

Looks ok. Can you ping upstream about including those fixes and/or rebuilding with fixed/newer auto* tools? 

cvs done.

gadmin-squid-0.1.0-0.4.fc8 has been submitted as an update for Fedora 8

gadmin-squid-0.1.0-0.4.fc9 has been submitted as an update for Fedora 9

This has been imported and built. Closing now.

gadmin-squid-0.1.0-0.4.fc8 has been pushed to the Fedora 8 stable repository.  If problems still persist, please make note of it in this bug report.

gadmin-squid-0.1.0-0.4.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.