Code review comment for lp://qastaging/~gz/bzrtools/shell_without_readline_719075

Revision history for this message
Aaron Bentley (abentley) wrote :

Making readline optional is fine, but please retain PEP8 import ordering-- readline should still be imported between os and shlex, not as if it was a third-party library.

I don't understand why you're explicitly disabling autocompletion. On platforms without readline, it won't work, but I don't think that incurs overhead.

Your test is broken. It passes (on Ubuntu Maverick) even if I revert the changes to shell.py. I believe it is sensitive to differences in python path.

I don't think it's sensible to test interactive functionality using a blackbox test. Tests are not required for "shell". Actually, all of bzrtools is a bit lax.

review: Needs Fixing

« Back to merge proposal