[Bug 466789] Review Request: jmol - an open-source Java viewer for chemical structures in 3D

bugzilla at redhat.com bugzilla at redhat.com
Mon Oct 13 20:53:33 UTC 2008


Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=466789


Peter Lemenkov <lemenkov at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |lemenkov at gmail.com




--- Comment #1 from Peter Lemenkov <lemenkov at gmail.com>  2008-10-13 16:53:32 EDT ---
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.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.




More information about the package-review mailing list