----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8305/#review13940 -----------------------------------------------------------
Hi Venkat, thank you very much for your time and effort working on this JIRA. I do have just couple of high level suggestions: client/src/main/java/org/apache/sqoop/client/core/ClientError.java <https://reviews.apache.org/r/8305/#comment29746> Sqoop is currently using common schema for exception all over the place and the named constants are quite important. I would suggest to leave the exception handling "as it is" for now and file additional JIRA for translating them. client/src/main/java/org/apache/sqoop/client/shell/CloneCommand.java <https://reviews.apache.org/r/8305/#comment29747> What about using static call MessageFormat.format here and on all other places? That way we would skip explicit object and explicit array creation. - Jarek Cecho On Dec. 1, 2012, 4:49 p.m., Venkat Ranganathan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8305/ > ----------------------------------------------------------- > > (Updated Dec. 1, 2012, 4:49 p.m.) > > > Review request for Sqoop and Jarek Cecho. > > > Description > ------- > > I have moved localizable strings to the client resources (those that are > descriptions, messages in general etc). Also consolidated constants to one > place and removed repetitive occurrences. > > 4 more files in utils need to be updated, but wanted to get this reviewed and > take that after this > > > Diffs > ----- > > client/src/main/java/org/apache/sqoop/client/core/ClientError.java fd3b97d > client/src/main/java/org/apache/sqoop/client/core/Constants.java 47c0547 > client/src/main/java/org/apache/sqoop/client/request/Request.java 1720507 > client/src/main/java/org/apache/sqoop/client/shell/CloneCommand.java > 847a6ad > > client/src/main/java/org/apache/sqoop/client/shell/CloneConnectionFunction.java > 21c41aa > client/src/main/java/org/apache/sqoop/client/shell/CloneJobFunction.java > b0e8d90 > client/src/main/java/org/apache/sqoop/client/shell/CreateCommand.java > 2453543 > > client/src/main/java/org/apache/sqoop/client/shell/CreateConnectionFunction.java > 734276d > client/src/main/java/org/apache/sqoop/client/shell/CreateJobFunction.java > 0b685bf > client/src/main/java/org/apache/sqoop/client/shell/DeleteCommand.java > bb09bf3 > > client/src/main/java/org/apache/sqoop/client/shell/DeleteConnectionFunction.java > ee2a1cf > client/src/main/java/org/apache/sqoop/client/shell/DeleteJobFunction.java > acc8e21 > client/src/main/java/org/apache/sqoop/client/shell/HelpCommand.java 03499d8 > client/src/main/java/org/apache/sqoop/client/shell/SetCommand.java 20c8090 > client/src/main/java/org/apache/sqoop/client/shell/SetOptionFunction.java > 3764306 > client/src/main/java/org/apache/sqoop/client/shell/SetServerFunction.java > daf1ff4 > client/src/main/java/org/apache/sqoop/client/shell/ShowCommand.java bd74253 > > client/src/main/java/org/apache/sqoop/client/shell/ShowConnectionFunction.java > 4e49288 > > client/src/main/java/org/apache/sqoop/client/shell/ShowConnectorFunction.java > a34c48c > > client/src/main/java/org/apache/sqoop/client/shell/ShowFrameworkFunction.java > 8dcf976 > client/src/main/java/org/apache/sqoop/client/shell/ShowJobFunction.java > ee8c63d > client/src/main/java/org/apache/sqoop/client/shell/ShowServerFunction.java > 039e28b > client/src/main/java/org/apache/sqoop/client/shell/ShowVersionFunction.java > 9e8c607 > client/src/main/java/org/apache/sqoop/client/shell/SqoopCommand.java > 9ae693e > client/src/main/java/org/apache/sqoop/client/shell/SqoopFunction.java > 200b3ee > client/src/main/java/org/apache/sqoop/client/shell/SqoopShell.java b2d05f4 > client/src/main/java/org/apache/sqoop/client/shell/SubmissionCommand.java > 39a2b31 > > client/src/main/java/org/apache/sqoop/client/shell/SubmissionStartFunction.java > 74ce905 > > client/src/main/java/org/apache/sqoop/client/shell/SubmissionStatusFunction.java > 4d683c0 > > client/src/main/java/org/apache/sqoop/client/shell/SubmissionStopFunction.java > 97628f7 > client/src/main/java/org/apache/sqoop/client/shell/UpdateCommand.java > 5bac209 > > client/src/main/java/org/apache/sqoop/client/shell/UpdateConnectionFunction.java > 4e55dba > client/src/main/java/org/apache/sqoop/client/shell/UpdateJobFunction.java > f7cdf26 > client/src/main/java/org/apache/sqoop/client/utils/ThrowableDisplayer.java > 45c78fb > client/src/main/resources/client-resource.properties 201efe9 > > Diff: https://reviews.apache.org/r/8305/diff/ > > > Testing > ------- > > Ran the SQOOP2 client tests and manually ran various client commands to make > sure that all commands have their localizable strings and constants properly > displayed apart from running all the unit tests. No new tests were added > > > Thanks, > > Venkat Ranganathan > >
