Bug 466789

Not a formal review:

* Summary should be simply "An open-source Java viewer for chemical structures in 3D" instead of "Jmol: an open-source Java viewer for chemical structures in 3D"

* You should note which svnver you're checking out. E.g. not only "svn co https://jmol.svn.sourceforge.net/svnroot/jmol/branches/v11_6/Jmol" but "svn co -r %{svnrel} https://jmol.svn.sourceforge.net/svnroot/jmol/branches/v11_6/Jmol"

* Use "svn export" instead of "svn co"

* Correct path for icon should be "http://wiki.jmol.org:81/images/Jmol_icon_128.png"

* About commented out "Requires:" - jmol doesn't requires java?

* Utility "install" ignores switch -c.

* No need to explicity create directories with mkdir in your case. You should use "-D" switch of "install" utility, e.g.

install -D -p -m 755 jmol %{buildroot}%{_bindir}/jmol

* You should use %{name} instead of jmol in your %install section (it simplifies copypasting, for example :). E.g.

install -D -p -m 755 %{name} %{buildroot}%{_bindir}/%{name}

* Conversion of documents must be done in more reliable way. I suggest the following:

for txtfile in README.txt COPYRIGHT.txt LICENSE.txt; do
	iconv -f ASCII -t UTF-8 $txtfile > $txtfile.new && mv $txtfile{.new,}
done


Probably there are others issues.

(In reply to comment #1)
> * Correct path for icon should be
> "http://wiki.jmol.org:81/images/Jmol_icon_128.png"

With this I get during build

/var/tmp/rpm-tmp.65853: line 52: unexpected EOF while looking for matching `"'
error: Bad exit status from /var/tmp/rpm-tmp.65853 (%install)

It seems that %{SOURCE2} somehow drops the first quotation mark.

Everything else should be fixed. Also I branched the documentation to separate packages. rpmlint output is clean for spec file and the three packages.

http://theory.physics.helsinki.fi/~jzlehtol/rpms/jmol.spec
http://theory.physics.helsinki.fi/~jzlehtol/rpms/jmol-11.6-2.10081svn.fc9.src.rpm

Okay, I'll take this and review it :-) More to follow....

Ping, how's the review coming along?

rpmlint is clean and nothing seems amiss with the build on my buildsys.
Spec file is good and clean and I can't see anything unowned by the package

I'm happy for this to go in.

APPROVED

New Package CVS Request
=======================
Package Name: jmol
Short Description: An open-source Java viewer for chemical structures in 3D
Owners: jussilehtola
Branches: F-9 F-10 EL-5
InitialCC:

(In reply to comment #5)
> rpmlint is clean and nothing seems amiss with the build on my buildsys.
> Spec file is good and clean and I can't see anything unowned by the package
> 
> I'm happy for this to go in.
> 
> APPROVED

Thanks for the review.

Please assign the package to yourself.

You did go through the review guidelines, right?

cvs done.

jmol-11.6-5.10137svn.fc9 has been submitted as an update for Fedora 9.
http://admin.fedoraproject.org/updates/jmol-11.6-5.10137svn.fc9

jmol-11.6-5.10137svn.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.