Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/10900 )
Change subject: IMPALA-1624: Allow toggling some command-line options inside impala-shell ...................................................................... Patch Set 2: (5 comments) http://gerrit.cloudera.org:8080/#/c/10900/2/shell/impala_shell.py File shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/10900/2/shell/impala_shell.py@131 PS2, Line 131: # Valid true value for some shell options nit: we typically finish comments with '.' in Impala I would also rephrase the sentence to something like "Strings that are interpreted as True for some boolean options." http://gerrit.cloudera.org:8080/#/c/10900/2/shell/impala_shell.py@177 PS2, Line 177: \ nit: please add a space before the last \ - I think that we do this everywhere in Impala. http://gerrit.cloudera.org:8080/#/c/10900/2/shell/impala_shell.py@743 PS2, Line 743: elif not self._handle_unset_shell_options(option): Please print a message when the unset succeeds. http://gerrit.cloudera.org:8080/#/c/10900/1/tests/shell/test_shell_interactive.py File tests/shell/test_shell_interactive.py: http://gerrit.cloudera.org:8080/#/c/10900/1/tests/shell/test_shell_interactive.py@125 PS1, Line 125: p2 = ImpalaShell() > Done Thanks for doing IMPALA-7286! Please also mention this change in the commit message. http://gerrit.cloudera.org:8080/#/c/10900/2/tests/shell/test_shell_interactive.py File tests/shell/test_shell_interactive.py: http://gerrit.cloudera.org:8080/#/c/10900/2/tests/shell/test_shell_interactive.py@94 PS2, Line 94: p.send_cmd("set delimiter=,") Can we keep the default delimiter in this test to cover more scenarios by the tests? -- To view, visit http://gerrit.cloudera.org:8080/10900 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id8d4487c24f24806223bfd5c54336914e3afd763 Gerrit-Change-Number: 10900 Gerrit-PatchSet: 2 Gerrit-Owner: Le Minh Nghia <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Jinchul Kim <[email protected]> Gerrit-Reviewer: Le Minh Nghia <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Fri, 13 Jul 2018 15:46:02 +0000 Gerrit-HasComments: Yes
