-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-openlmi.rhcloud.com/r/670/#review845
-----------------------------------------------------------
openlmi-mof-register
<
http://reviewboard-openlmi.rhcloud.com/r/670/#comment453>
Bash has native print option to print current time:
printf "%(%Y-%m-%d)T" -1
openlmi-mof-register
<
http://reviewboard-openlmi.rhcloud.com/r/670/#comment450>
log "COMMAND:" "$@"
looks more natural for me. Using this, you will pass as 1st argument the type and
other arguments will be command (with parameters) what you like.
openlmi-mof-register
<
http://reviewboard-openlmi.rhcloud.com/r/670/#comment451>
Using eval is very error prone; the command injection is very easy to do, we must be
careful.
We must also take care that parameters doesn't have any shell functionality.
To be honest, I don't like it.
openlmi-mof-register
<
http://reviewboard-openlmi.rhcloud.com/r/670/#comment452>
Also split to log "CODE:" "$?"
openlmi-mof-register
<
http://reviewboard-openlmi.rhcloud.com/r/670/#comment454>
This will not work. You are checking return value from log_command, it's return
value is return value from last executed command, which is log, in turn it is printf.
openlmi-mof-register
<
http://reviewboard-openlmi.rhcloud.com/r/670/#comment455>
log STARTING "$0" "$@"
openlmi-providers.spec
<
http://reviewboard-openlmi.rhcloud.com/r/670/#comment456>
You can encapsulate the all commands to {} and redirect once. Something like:
if ...
{
} >>...
fi
or do the redirection for the if command:
if...
fi >>...
It's simpler, readable and less error prone.
- Roman Rakus
On Aug. 1, 2013, 1:19 p.m., Radek Novacek wrote:
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-openlmi.rhcloud.com/r/670/
-----------------------------------------------------------
(Updated Aug. 1, 2013, 1:19 p.m.)
Review request for OpenLMI Developers.
Repository: openlmi-providers
Description
-------
Add logging to the openlmi-mof-register script
Each command in the registration script is now logged into
/var/log/openlmi-install.log file.
Diffs
-----
openlmi-mof-register 4e60e31786e22b063e4c3a42116e05038f5275c3
openlmi-providers.spec 0552a8cc01c8c3979a2304463f0a663fcc993a95
Diff:
http://reviewboard-openlmi.rhcloud.com/r/670/diff/
Testing
-------
Thanks,
Radek Novacek
_______________________________________________
openlmi-reviews mailing list
openlmi-reviews(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/openlmi-reviews