> %define _bindir /usr/bin
> %define _mandir /usr/share/man

Why? Those macros are used even on RHEL5.

> rm -rf $RPM_BUILD_ROOT
a) this should be at the beggining of %install and not prep, b) not necessary unless you target EPEL

Use macros consitently. I.e. %{buildroot} instead of $RPM_BUILD_ROOT.

> gzip $RPM_BUILD_ROOT/%{_mandir}/man1/metapixel.1
Not necessary. rpm will compress it automatically. Actually rpm can use even different compress format e.g. xz
Therefore
  %files
  %{_mandir}/man1/metapixel.1.gz
should be rather
  %files
  %{_mandir}/man1/metapixel.1*

> rm -rf %{buildroot}
> %defattr(-, root, root)
Both not needed unless you target EPEL

- Dist tag must be present.

- If (and only if) the source package includes the text of the license(s)
  in its own file, then that file, containing the text of the license(s)
  for the package is included in %license.
  Note: License file COPYING is marked as %doc instead of %license
  See:
  http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text


Side note, this package already exist in dist-git, but is retired, so you would not need new_package request, just change request.

Please check the license tag so it correctly follow reality:
GPL (v2 or later) (with incorrect FSF address)
----------------------------------------------
metapixel-1.0.2/convert.c
metapixel-1.0.2/getopt.c
metapixel-1.0.2/getopt.h
metapixel-1.0.2/getopt1.c
metapixel-1.0.2/imagesize.c
metapixel-1.0.2/metapixel.c
metapixel-1.0.2/metapixel.h
metapixel-1.0.2/pools.c
metapixel-1.0.2/pools.h
metapixel-1.0.2/rwimg/readimage.c
metapixel-1.0.2/rwimg/readimage.h
metapixel-1.0.2/rwimg/rwgif.c
metapixel-1.0.2/rwimg/rwgif.h
metapixel-1.0.2/rwimg/rwjpeg.c
metapixel-1.0.2/rwimg/rwjpeg.h
metapixel-1.0.2/rwimg/rwpng.c
metapixel-1.0.2/rwimg/rwpng.h
metapixel-1.0.2/rwimg/writeimage.c
metapixel-1.0.2/rwimg/writeimage.h
metapixel-1.0.2/vector.c
metapixel-1.0.2/vector.h
metapixel-1.0.2/zoom.c
metapixel-1.0.2/zoom.h

LGPL (v2 or later) (with incorrect FSF address)
-----------------------------------------------
metapixel-1.0.2/allocator.c
metapixel-1.0.2/allocator.h
metapixel-1.0.2/lispreader.c
metapixel-1.0.2/lispreader.h

You may even wont to contact upstream so they update their license file.

BTW http://www.complang.tuwien.ac.at/schani/metapixel/metapixel-1.0.2.tar.gz
produce 404 Not Found.

metapixel.x86_64: W: name-repeated-in-summary C Metapixel
You should not repeat the name in summary.

metapixel.x86_64: W: incoherent-version-in-changelog 1.0.2 ['1.0.2-1', '1.0.2-1']
The version in changelog should include release number as well.

metapixel.x86_64: W: spurious-executable-perm /usr/share/man/man1/metapixel.1.gz

metapixel.src:32: W: setup-not-quiet
You should pass -q to %setup macro (without -q it should be used only for debugging).

sorry about that, I didn't even think to check dist-git.  I'll just unretire it.  Thanks

Package Change Request
======================
Package Name: metapixel
New Branches: master
Owners: nhorman

please unretire this package, I have fixes to allow it to build again.

I'm afraid that you need to go through review process anyway because this package is retired for long time. See
https://fedoraproject.org/wiki/Orphaned_package_that_need_new_maintainers#Claiming_Ownership_of_a_Retired_Package

Clearing cvs flag.

Spec URL: http://people.redhat.com/nhorman/metapixel.spec
SRPM URL: http://people.redhat.com/nhorman/metapixel-1.0.2-1.src.rpm

Ok, new packages/spec available for review

Change notes:

*rpmlint cleanups.  Should be clean save for a false positive spelling issue (photomosaic isn't a recognized word)

* License Address changes to properly reflect current FSF address (I'm preparing an upstream patch for this as well)

* Removed needless macros

* Enforced consistent use of RPM_BUILD_ROOT

* Fixed man page permission and compression

* Added dist tag

* Fixed spec file to list COPYING as %license

* Fixed spec file license tag to reflect reality (LGPLv2+ and GPLv2+), though that should be fixed upstream too, given that this build produces no DSO's, making the LGPL largely moot.


I've also sent a note to devel announcing my intent to unretire this package, so pending your review, we should be good to start with the admin cvs requests.

SPEC in #7 differs from SPEC file included in SRPM in #7.

> %{_mandir}/man1/metapixel.1
This should be:
%{_mandir}/man1/metapixel.1*

> Requires: libpng
> Requires: libjpeg
> Requires: giflib

This is not needed. Rpm automatically guess this dependency and generate e.g for libjpeg those deps:
  libjpeg.so.62()(64bit)
  libjpeg.so.62(LIBJPEG_6.2)(64bit)
which is more precise than listing libjpeg explicitely.

>%changelog
> * Wed Aug 26 2015 Neil Horman <nhorman> - 1.0.2

As I said, you should user release in changelog entry as well. Therefore:
* Wed Aug 26 2015 Neil Horman <nhorman> - 1.0.2-1

Spec URL: http://people.redhat.com/nhorman/metapixel.spec
SRPM URL: http://people.redhat.com/nhorman/metapixel-1.0.2-1.src.rpm

All fixed, new version available, thanks!

(In reply to Neil Horman from comment #9)
> All fixed, new version available, thanks!

Well,
- this package doesn't honor CFLAGS:
...
gcc -I/sw/include -I/usr/X11R6/include -I/usr/X11R6/include/X11 -I. -Irwimg -Wall -O2   -DMETAPIXEL_VERSION=\"1.0.2\" -DRWIMG_JPEG -DRWIMG_PNG -DRWIMG_GIF -c metapixel.c
...

- this package doesn't honor ans installation paths. 
This is the origin why "make install" appears broken to you and likely is why you resorted to manual installation.
 
- this package's "release" lacks %{?dist}

- there still are superflous deps:
# rpmlint metapixel-1.0.2-1.x86_64.rpm 
metapixel.x86_64: E: explicit-lib-dependency giflib
metapixel.x86_64: E: explicit-lib-dependency libjpeg
metapixel.x86_64: E: explicit-lib-dependency libpng

Ralf, you're two revisions behind in your reviews.  The deps have been fixed already

Though you're correct about the missed cflags and install paths.  The makefile honors the paths, but the spec file was taken from the upstream project, and I neglected to fix that up. will do so

Spec URL: http://people.redhat.com/nhorman/metapixel.spec
SRPM URL: http://people.redhat.com/nhorman/metapixel-1.0.2-1.src.rpm

There, cflags and install paths fixed.

(In reply to Neil Horman from comment #11)
> Ralf, you're two revisions behind in your reviews.
It's what I found in http://people.redhat.com/nhorman/metapixel-1.0.2-1.src.rpm shortly after comment #9 arrived by email.

As you did not increment %release, it's impossible for to tell if the version I downloaded is the right one => Please increment %release each time you change something in your spec. Not doing so adds avoidable complications to reviews.

(In reply to Neil Horman from comment #12)
> Spec URL: http://people.redhat.com/nhorman/metapixel.spec
> SRPM URL: http://people.redhat.com/nhorman/metapixel-1.0.2-1.src.rpm
> 
> There, cflags and install paths fixed.
Well, you seem to have updated http://people.redhat.com/nhorman/metapixel.spec 
but not http://people.redhat.com/nhorman/metapixel-1.0.2-1.src.rpm

- Impossible to review this package, because *.spec does not match the srpm.
=>  metapixel-copyright.patch and metapixel-install.patch are missing.

- If metapixel-copyright.patch is what I presume it is (Changing the FSF address) then it's superfluous, because it's legally irrelevant.

- MUSTFIX: Add %{?dist} to release

- make PREFIX=... is pretty much useless.
You need to add BINDIR and MANDIR as well.

- Requires: perl also seems superfluous. Rpm automatically adds appropriate R: to packages containing perl scripts

sorry, typoed the srpm address

Spec URL: http://people.redhat.com/nhorman/metapixel.spec
SRPM URL: http://people.redhat.com/nhorman/metapixel-1.0.2-2.fc21.src.rpm

even bumped the release for you :)

Regarding the copyright patch, it is what you presume, and while I agree its legally irrelevant, its also easy to do, so instead of arguing about it, I just updated it.

%{dist} tag has been fixed for some time, above was a typo.

BINDIR and MANDIR are set appropriately in the makefile, no need to do that in the spec file.

Regarding perl, it may be superfulous, but its also not getting a complaint from rpmlint on the subject, nor are the packaging guidelines explicit on the subject.  I'd just as soon leave it in place

metapixel-copyright.patch is not necessary. It is just enough to contact upstream and notify them. However it is neither blocker for review.

Everything else was addressed (I agree with Neil on BINDIR and MANDIR and perl issues). Therefore:

APPROVED

Package Change Request
======================
Package Name: metapixel
New Branches: master f23
Owners: nhorman

please unretire the metapixel package and assign me as the owner

Git done (by process-git-requests).

(In reply to Miroslav Suchý from comment #17)

> Everything else was addressed (I agree with Neil on BINDIR and MANDIR and
> perl issues). Therefore:
I do not agree. Though issues are minor, they will very likely hit you in future.

> APPROVED
Well, you don't want to know what I think of this.

metapixel-1.0.2-2.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2015-15527

metapixel-1.0.2-2.fc23 has been pushed to the Fedora 23 testing repository. If problems still persist, please make note of it in this bug report.\nIf you want to test the update, you can install it with \n su -c 'yum --enablerepo=updates-testing update metapixel'. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2015-15527

metapixel-1.0.2-2.fc23 has been pushed to the Fedora 23 stable repository. If problems still persist, please make note of it in this bug report.