----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27566/#review60811 -----------------------------------------------------------
Few requests to improve docs, otherwise LGTM! beeline/src/java/org/apache/hive/beeline/BeeLineOpts.java <https://reviews.apache.org/r/27566/#comment102221> extra ; LICENSE <https://reviews.apache.org/r/27566/#comment102583> extra word "contains" beeline/src/java/org/apache/hive/beeline/BeeLineCommandCompleter.java <https://reviews.apache.org/r/27566/#comment102619> nit: add @Override beeline/src/java/org/apache/hive/beeline/BeeLineCommandCompleter.java <https://reviews.apache.org/r/27566/#comment102667> While we're here: Could we rename "compl" to "embeddedCompleters"? Also, it's more readable if we get rid of "comps" and just add it directly (since it's not used anywhere else) beeline/src/java/org/apache/hive/beeline/BeeLineOpts.java <https://reviews.apache.org/r/27566/#comment102671> extra ; beeline/src/java/org/apache/hive/beeline/BooleanCompleter.java <https://reviews.apache.org/r/27566/#comment102672> nit: extra lines beeline/src/java/org/apache/hive/beeline/ReflectiveCommandHandler.java <https://reviews.apache.org/r/27566/#comment102637> While we're here, could we please add javadoc. In particular, clarify that 'cmds' is an array of alternative names for the same command. And that the first one is always chosen for display purposes and to lookup help documentation from BeeLine.properties file. cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java <https://reviews.apache.org/r/27566/#comment102680> move this under jline imports cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java <https://reviews.apache.org/r/27566/#comment102682> let's rename this to strCompleter cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java <https://reviews.apache.org/r/27566/#comment102676> Import jline.console.completer.ArgumentCompleter.ArgumentDelimiter cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java <https://reviews.apache.org/r/27566/#comment102681> let's rename this to argCompleter cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java <https://reviews.apache.org/r/27566/#comment102687> let's rename to customCompleter cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java <https://reviews.apache.org/r/27566/#comment102691> setCompleter cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java <https://reviews.apache.org/r/27566/#comment102692> confCompleter hcatalog/hcatalog-pig-adapter/pom.xml <https://reviews.apache.org/r/27566/#comment102695> 2.12 ? ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezJobMonitor.java <https://reviews.apache.org/r/27566/#comment102697> move down with other jline imports - Mohit Sabharwal On Nov. 12, 2014, 2:43 a.m., cheng xu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27566/ > ----------------------------------------------------------- > > (Updated Nov. 12, 2014, 2:43 a.m.) > > > Review request for hive. > > > Repository: hive-git > > > Description > ------- > > HIVE-8609: move beeline to jline2 > The following will be changed: > * MultiCompletor-> AggregateCompleter > * SimpleCompletor->StringsCompleter > * Terminal.getTerminalWidth() -> Terminal.getWidth() > * Terminal is an interface now; -> use TerminalFactory to get instances of a > Terminal > * String -> CharSequence > > > Diffs > ----- > > LICENSE 2885945 > beeline/src/java/org/apache/hive/beeline/AbstractCommandHandler.java > a9479d5 > beeline/src/java/org/apache/hive/beeline/BeeLine.java 8539a41 > beeline/src/java/org/apache/hive/beeline/BeeLineCommandCompleter.java > PRE-CREATION > beeline/src/java/org/apache/hive/beeline/BeeLineCommandCompletor.java > 52313e6 > beeline/src/java/org/apache/hive/beeline/BeeLineCompleter.java PRE-CREATION > beeline/src/java/org/apache/hive/beeline/BeeLineCompletor.java c6bb4fe > beeline/src/java/org/apache/hive/beeline/BeeLineOpts.java f73fb44 > beeline/src/java/org/apache/hive/beeline/BooleanCompleter.java PRE-CREATION > beeline/src/java/org/apache/hive/beeline/BooleanCompletor.java 3e88c53 > beeline/src/java/org/apache/hive/beeline/ClassNameCompleter.java > PRE-CREATION > beeline/src/java/org/apache/hive/beeline/CommandHandler.java bab1778 > beeline/src/java/org/apache/hive/beeline/Commands.java 7e366dc > beeline/src/java/org/apache/hive/beeline/DatabaseConnection.java ab67700 > beeline/src/java/org/apache/hive/beeline/ReflectiveCommandHandler.java > 2b957f2 > beeline/src/java/org/apache/hive/beeline/SQLCompleter.java PRE-CREATION > beeline/src/java/org/apache/hive/beeline/SQLCompletor.java 844b9ae > beeline/src/java/org/apache/hive/beeline/TableNameCompletor.java bc0d9be > cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java 0ccaacb > cli/src/test/org/apache/hadoop/hive/cli/TestCliDriverMethods.java 88a37d5 > hcatalog/hcatalog-pig-adapter/pom.xml 2d959e6 > pom.xml ec8c4fe > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezJobMonitor.java dea3460 > > Diff: https://reviews.apache.org/r/27566/diff/ > > > Testing > ------- > > > Thanks, > > cheng xu > >
