Jim Apple has posted comments on this change. ( http://gerrit.cloudera.org:8080/8613 )
Change subject: IMPALA-4506: Do not display some intro message if --quiet is set ...................................................................... Patch Set 3: (4 comments) http://gerrit.cloudera.org:8080/#/c/8613/3/shell/impala_shell.py File shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/8613/3/shell/impala_shell.py@1353 PS3, Line 1353: def get_intro(options): Please add a docstring to new functions. http://gerrit.cloudera.org:8080/#/c/8613/3/shell/impala_shell.py@1359 PS3, Line 1359: "not secured by TLS.\nALL PASSWORDS WILL BE SENT IN THE CLEAR TO IMPALA.\n") nit: long line http://gerrit.cloudera.org:8080/#/c/8613/3/shell/impala_shell.py@1362 PS3, Line 1362: intro += REFRESH_AFTER_CONNECT_DEPRECATION_WARNING nit, not your bug (already present in code): Can you make the last character of the return value (if there is one) always '\n' or never '\n'? http://gerrit.cloudera.org:8080/#/c/8613/3/shell/impala_shell.py@1364 PS3, Line 1364: else: : return "" You could remove a level of indentation for the bulk of the function by making the first two non-docstring lines be if not options.verbose: return "" -- To view, visit http://gerrit.cloudera.org:8080/8613 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I19c6d00dfbbe805ee9c525b72eb5703840e2f582 Gerrit-Change-Number: 8613 Gerrit-PatchSet: 3 Gerrit-Owner: Kim Jin Chul <jinc...@gmail.com> Gerrit-Reviewer: Jim Apple <jbapple-imp...@apache.org> Gerrit-Reviewer: Kim Jin Chul <jinc...@gmail.com> Gerrit-Reviewer: Philip Zeyliger <phi...@cloudera.com> Gerrit-Comment-Date: Mon, 27 Nov 2017 14:56:14 +0000 Gerrit-HasComments: Yes
