Script Review

[Hunki]

I have this script which I would want you all to review. Please see attachment for further details.

Thanks,
hunki

[James R. Ferguson]

Hi Hunki:

So what would you like to know?

Instead of in-lining repetetive commands (e.g. 'mkdir') I'd create a forlist like:

for DIR in ...
do
mkdir -p ${DIR}
done

The same would/could apply for your 'rcp' list.

Doing this would allow you (later, if necessary) to encapsulate the 'mkdir' in a called subroutine with other code.

Regards!

...JRF...

[Steven E. Protter]

Shalom hunki,

It looks good to me.

Its a little long and inflexible as JRF points out.

Going to alist lets you change the list without breaking the script accidently during vi editing.

I'd also say paying the performance penalty of using scp instead of rcp would protect the data stream with encrytion were you so inclined.

SEP

[A. Clay Stephenson]

userRunning=`/usr/ucb/whoami`
better:
userRunning=$(/usr/ucb/whoami)

and so on for all of your `commands'

while not strictly necessary, none of your variables are typeset. If you typeset nowhere else, at least do it within functions
to prevent side effects.

repository=csdev91
better:
typeset repository=csdev91

None of your variables are enclosed in {}'s.

if [[ $userRunning = aqprod && $host = ainsmsa ]]
better:
if [[ ${userRunning} = aqprod && ${host} = ainsmsa ]]
and better still so that null variables won't crash your script:
if [[ "${userRunning}" = "aqprod" && "${host}" = "ainsmsa" ]]

In most cases, the {}'s are not required but they never cause harm. It's best to get into the habit of always surrounding variables with {}'s so that the code inside your head never branches.

Note that your use of rsh is not portable because on some platforms "rsh" should be "remsh" and "rsh" is actually the restricted shell. It's better to test if "remsh" is available and executable and use it and if not then use rsh. This suggests that you need a variable for the executable.

[john korterman]

Hi Hunki,

it is also a good idea to check that the cd you make before the rm command actually succeeds, e.g.:

cd $SACE
if [ "$?" = "0" ]
then

else

fi

if you omit a check like the above and the cd command fails, rm will instead work in the current directory.


regards,
John K.