----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5626/#review8720 -----------------------------------------------------------
Some comments/suggestions below: /branches/sqoop2/client/pom.xml <https://reviews.apache.org/r/5626/#comment18396> It looks like ExpectJ is under GNU LGPL license. However, because of the Apache licensing policy, it is one of the excluded licenses and must not apply to any software within an Apache product. /branches/sqoop2/client/src/main/java/org/apache/sqoop/client/shell/SetServerFunction.java <https://reviews.apache.org/r/5626/#comment18397> Rather than adding "q" as an option here, how about make it explicit in Question classes? That is, spell it out in the prompt, such as "Select [....] or type "q" to quit:". /branches/sqoop2/client/src/main/java/org/apache/sqoop/client/shell/ShowServerFunction.java <https://reviews.apache.org/r/5626/#comment18450> How about factor out this loop as a method (eg. addOptions) on MultiChoiceQuestion since it is used at many places? /branches/sqoop2/client/src/main/java/org/apache/sqoop/client/shell/interactivesession/MultiChoiceQuestion.java <https://reviews.apache.org/r/5626/#comment18451> It would be nice if the options can be sorted before being displayed later. - Bilung Lee On June 28, 2012, 1:14 a.m., Cheolsoo Park wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/5626/ > ----------------------------------------------------------- > > (Updated June 28, 2012, 1:14 a.m.) > > > Review request for Sqoop and Bilung Lee. > > > Description > ------- > > If a command does not have full options specified, instead of displaying full > help, the shell could get into an interactive session to gather the options > one by one from the user. > > > This addresses bug SQOOP-494. > https://issues.apache.org/jira/browse/SQOOP-494 > > > Diffs > ----- > > /branches/sqoop2/client/pom.xml 1354162 > > /branches/sqoop2/client/src/main/java/org/apache/sqoop/client/shell/SetServerFunction.java > 1354162 > > /branches/sqoop2/client/src/main/java/org/apache/sqoop/client/shell/ShowConnectorFunction.java > 1354162 > > /branches/sqoop2/client/src/main/java/org/apache/sqoop/client/shell/ShowServerFunction.java > 1354162 > > /branches/sqoop2/client/src/main/java/org/apache/sqoop/client/shell/ShowVersionFunction.java > 1354162 > > /branches/sqoop2/client/src/main/java/org/apache/sqoop/client/shell/SqoopFunction.java > 1354162 > > /branches/sqoop2/client/src/main/java/org/apache/sqoop/client/shell/SqoopShell.java > 1354162 > > /branches/sqoop2/client/src/main/java/org/apache/sqoop/client/shell/interactivesession/MultiChoiceQuestion.java > PRE-CREATION > > /branches/sqoop2/client/src/main/java/org/apache/sqoop/client/shell/interactivesession/Question.java > PRE-CREATION > > /branches/sqoop2/client/src/main/java/org/apache/sqoop/client/shell/interactivesession/ShortAnswerQuestion.java > PRE-CREATION > > /branches/sqoop2/client/src/main/java/org/apache/sqoop/client/shell/interactivesession/YesNoQuestion.java > PRE-CREATION > > /branches/sqoop2/client/src/test/java/org/apache/sqoop/client/TestInteractiveSession.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/5626/diff/ > > > Testing > ------- > > Added two unit tests for "set server" and "show server". > > > Thanks, > > Cheolsoo Park > >
