Mark J. Nelson wrote:
Howdy--
Heya,
http://cr.opensolaris.org/~mjnelson/onnv-bugfixes/
This wad addresses
6830663 webrev -n -U, but without -o, no longer works after 6820421
Thanks for the quick response.
You might want to call external commands the ksh-way, using the $()
brackets, lines 2274, 2285, 2286 (and comments at 2267, 2281, 2282).
The command at 2275 should be performed using && and checked if
codemgr_ws is non-empty afterwards I believe.
Also, you might consider adding some sort of ASSERT at line 2448 to
check CWS is indeed set/non-empty to support the comment at 2444-2447.
The whole block at 2263-2314 makes sure CWS is always set (however,
checking if CWS is a directory should probably be done for the else
branch at 2308 since CODEMGR_WS grabbed from the environment could be
bogus) but preventing future code from breaking things does not hurt.
Otherwise this looks good.
2 comments, not related to this fix:
- going through the script, it seems it has grown over the limit of
maintainability - it should probably be split into multiple files
according to the functionality (HTML/PS stuff, misc, SCM specific stuff).
- for the 3 supported SCMs the script is calling the commands
(workspace for Teamware, hg for Mercurial, svn for Subversion) directly.
Checking for the presence of all the commands is not the right thing to
do (not everyone has access to Teamware tools) but the script should be
more robust in case the commands are not present (lines 2274, 2286, 2297).
v.
_______________________________________________
tools-discuss mailing list
tools-discuss@opensolaris.org
