Code review comment for lp:~pitti/autopilot/print-tree

Revision history for this message
Thomi Richards (thomir-deactivatedaccount) wrote :

Hi,

Nice branch! I have a few suggestions though:

1)

16 + def print_tree(self, output=None, maxdepth=None, _curdepth=0):

Instead of making the default value for 'output' be 'None', why not make it 'sys.stdout' ? That way, you can delete this:

39 + if output is None:
40 + output = sys.stdout

And the docs should say that output should be a "file-like object that supports the 'write' method. That way you can delete these lines as well:

41 + elif isinstance(output, six.string_types):
42 + output = open(output, 'w')

I'm not sure that using four spaces as an indentation is a good idea - trees can get pretty large... have you tried this one a bug tree like Unity 7? Perhaps 2 spaces would be better? I'm really not sure either way - but I 'd like to see some larger output....

review: Needs Fixing

« Back to merge proposal