----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://codereview.secondlife.com/r/213/#review471 -----------------------------------------------------------
Ship it! This change looks correct and well done. (Didn't test it yet, though.) Thanks! I especially like the aspect that logging is moved from the callers to the callee, which makes the affected code cleaner. I have some ideas on how to make this even better: * Make it easier to see where the command starts, e.g. by adding a semicolon after the word "command" or by adding some color. * In the dry-run case, should the log message indicate that nothing actually happens? (e.g. have it say "would execute" instead of "executing" if dry_run is true) Both these suggestions would change expected behavior, so they are out of the scope of this issue. I'll file separate jira issues for them. - Boroondas On March 16, 2011, 8:19 p.m., Oz Linden wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://codereview.secondlife.com/r/213/ > ----------------------------------------------------------- > > (Updated March 16, 2011, 8:19 p.m.) > > > Review request for Viewer and Alain Linden. > > > Summary > ------- > > Extended the __call__ method of Executable to pass in the command type as a > string and the dry_run flag so that the logging of the command being run > could use the infrastructure there to exactly assemble the options and > arguments for the logging of the command to be run. The callers then don't > need to do the logging separately or do anything with dry_run other than pass > it down to the executable. > > > This addresses bug open-45. > http://jira.secondlife.com/browse/open-45 > > > Diffs > ----- > > autobuild/autobuild_tool_build.py abc1014d5ad6 > autobuild/autobuild_tool_configure.py abc1014d5ad6 > autobuild/executable.py abc1014d5ad6 > > Diff: http://codereview.secondlife.com/r/213/diff > > > Testing > ------- > > > Thanks, > > Oz > >
_______________________________________________ Policies and (un)subscribe information available here: http://wiki.secondlife.com/wiki/OpenSource-Dev Please read the policies before posting to keep unmoderated posting privileges
