Bug 461897

Looks very well.

Please remove the leading blank line in %description, I am not sure it would look well in GUI tools. Description probably needs some more love -- the fact that the package is free doesn't need to be mentioned there (redundant, see License tag) and "X windows" is correctly spelled without the trailing "s" and with "w" capitablized: "X Window", "X Window System", or just "X11".

It's not clear what does this line do, and why does it belong to %prep. Please explain and add a comment:
make -f Makefile.cvs dist

What's "%{_datadir}/apps"? Would not "%{_datadir}/raytracer" be more appropriate? In case not, please ensure you depend on the package that creates it or own it.
%{_datadir}/apps/raytracer/

(In reply to comment #1)
> Please remove the leading blank line in %description, I am not sure it would
> look well in GUI tools. 

Done.

> Description probably needs some more love -- the fact
> that the package is free doesn't need to be mentioned there (redundant, see
> License tag) and "X windows" is correctly spelled without the trailing "s" and
> with "w" capitablized: "X Window", "X Window System", or just "X11".

Yep, sorry for that, I've just copied and pasted it from the author's webpages.

> It's not clear what does this line do, and why does it belong to %prep. Please
> explain and add a comment:
> make -f Makefile.cvs dist

This is now not necessary anymore thanks to fast upstream reaction. Also, my patch for 64bit systems has been merged and new version released (how nice!:).
  
> What's "%{_datadir}/apps"? Would not "%{_datadir}/raytracer" be more
> appropriate? In case not, please ensure you depend on the package that creates
> it or own it.
> %{_datadir}/apps/raytracer/

Look like a standard path for KDE apps. It should be ok as opticalraytracer depends on kdelibs3 which in turn depends on kde-filesystem which owns %{datadir}/apps. The same applies for %{docdir}/HTML/en where newly some files are stored too.

Note: rpmlint is silent on spec file and SRPM, but not on RPMs:

>rpmlint -i opticalraytracer-1.2-1.fc9.x86_64.rpm
opticalraytracer.x86_64: W: dangling-relative-symlink /usr/share/doc/HTML/en/raytracer/common ../common
The relative symbolic link points nowhere.

I don't know whether I can avoid this somehow, the pointed directory is owned by kdelibs-common which is also a dependency of kdelibs3, hence this shouldn't be IMHO a problem.

New SPEC file: http://mjakubicek.fedorapeople.org/opticalraytracer/opticalraytracer.spec
New SRPM: http://mjakubicek.fedorapeople.org/opticalraytracer/opticalraytracer-1.2-1.fc9.src.rpm
(there are also x68_64 RPMs in the same dir)

> (In reply to comment #1)
> > Description probably needs some more love -- the fact
> > that the package is free doesn't need to be mentioned there (redundant, see
> > License tag) and "X windows" is correctly spelled without the trailing "s" and
> > with "w" capitablized: "X Window", "X Window System", or just "X11".
> 
> Yep, sorry for that, I've just copied and pasted it from the author's webpages.

Similarly to %description, I suggest you change the Comment= in .desktop file to somthing more sensible (such as "Utility that analyzes the system of lenses" (may apply to Summary as well):

Comment=OpticalRayTracer is a free (GPL) Linux (Xwindows GUI-based) utility that analyzes systems of lenses.(In reply to comment #2)

This is definitely not a blocker though.

> This is now not necessary anymore thanks to fast upstream reaction. Also, my
> patch for 64bit systems has been merged and new version released (how nice!:).

Thanks for that!

> > What's "%{_datadir}/apps"? Would not "%{_datadir}/raytracer" be more
> > appropriate? In case not, please ensure you depend on the package that creates
> > it or own it.
> > %{_datadir}/apps/raytracer/
> 
> Look like a standard path for KDE apps. It should be ok as opticalraytracer
> depends on kdelibs3 which in turn depends on kde-filesystem which owns
> %{datadir}/apps. The same applies for %{docdir}/HTML/en where newly some files
> are stored too.
> 
> Note: rpmlint is silent on spec file and SRPM, but not on RPMs:
> 
> >rpmlint -i opticalraytracer-1.2-1.fc9.x86_64.rpm
> opticalraytracer.x86_64: W: dangling-relative-symlink
> /usr/share/doc/HTML/en/raytracer/common ../common
> The relative symbolic link points nowhere.
> 
> I don't know whether I can avoid this somehow, the pointed directory is owned
> by kdelibs-common which is also a dependency of kdelibs3, hence this shouldn't
> be IMHO a problem.

That's fine then.

The package looks perfect now; thanks and sorry for the delay;

APPROVED

Thanks for the review (the .desktop and summary will be updated of course)!

New Package CVS Request
=======================
Package Name: opticalraytracer
Short Description: Utility that analyzes systems of lenses
Owners: mjakubicek
Branches: F-8 F-9
InitialCC: mmahut

cvs done

opticalraytracer-1.2-1.fc9 has been submitted as an update for Fedora 9.
http://admin.fedoraproject.org/updates/opticalraytracer-1.2-1.fc9

opticalraytracer-1.2-1.fc8 has been submitted as an update for Fedora 8.
http://admin.fedoraproject.org/updates/opticalraytracer-1.2-1.fc8

opticalraytracer-1.2-1.fc9 has been pushed to the Fedora 9 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update opticalraytracer'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F9/FEDORA-2008-8701

opticalraytracer-1.2-1.fc8 has been pushed to the Fedora 8 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update opticalraytracer'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F8/FEDORA-2008-8743

opticalraytracer-1.2-1.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.

opticalraytracer-1.2-1.fc8 has been pushed to the Fedora 8 stable repository.  If problems still persist, please make note of it in this bug report.