Red Hat Bugzilla – Bug 461897
Review Request: opticalraytracer - OpticalRayTracer is a Linux utility that analyzes systems of lenses
Last modified: 2008-10-30 08:50:33 EDT
Spec URL: http://mjakubicek.fedorapeople.org/opticalraytracer/opticalraytracer.spec SRPM URL: http://mjakubicek.fedorapeople.org/opticalraytracer/opticalraytracer-1.1-1.fc9.src.rpm Description: OpticalRayTracer is a free (GPL) Linux (X windows GUI-based) utility that analyzes systems of lenses. It uses optical principles and a virtual optical bench to predict the behavior of many kinds of ordinary and exotic lens types. OpticalRayTracer includes an advanced, easy-to-use interface that allows the user to rearrange the optical configuration by simply dragging lenses around using the mouse. Koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=819744
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.