#11503: Make new spkg to install Jmol in SAGE_LOCAL/share
------------------------+---------------------------------------------------
Reporter: gutow | Owner: gutow
Type: task | Status: needs_work
Priority: major | Milestone: sage-5.0
Component: notebook | Keywords: sd31,Jmol, flask
Work_issues: | Upstream: N/A
Reviewer: | Author: gutow
Merged: | Dependencies: flask notebook
------------------------+---------------------------------------------------
Changes (by ddrake):
* status: needs_review => needs_work
Comment:
I'm looking this over, and I have some more minor nits to pick with spkg-
install. Here's an idiom I see a couple times:
{{{
TEMPDIR=something
cd "$TEMPDIR"
do some stuff
...
}}}
Two things bother me about that: (1) you call it `TEMPDIR`, which implies
to me that it's a temporary directory used for some scratch work, but
you're talking about an existing directory with important files in it. (2)
You never use the `TEMPDIR` variable, except in the `cd` command...so why
not just `cd` to the directory?
Also, I think it's more readable to just put the paths into commands,
rather than `cd`ing around. I would prefer to see
{{{
cp $SAGE_FOO/dir/file.txt $SAGE_BAR/dir/dir2/
}}}
rather than
{{{
cd $SAGE_FOO/dir
cp file.txt $SAGE_BAR/dir/dir2/
}}}
It makes it easier to read the script, since you don't have to remember
what the working directory is. Also, you sometimes change directories
inside an if-then-else, so the working directory depends on what happened
there!
Finally, at the end of `spkg-install`, you have "PACKAGE NAME" instead of
Jmol!
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/11503#comment:15>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/sage-trac?hl=en.
