----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55132/#review160508 -----------------------------------------------------------
src/java/org/apache/sqoop/manager/Db2Manager.java (line 144) <https://reviews.apache.org/r/55132/#comment231645> I might miss a point here but could you please clarify what does "$$$$$ shiju" stand for in the message? Also, writing to STDOUT won't appear in the log, you should use org.apache.commons.logging.Log and org.apache.commons.logging.LogFactory as you did it in the added test case. src/java/org/apache/sqoop/manager/Db2Manager.java (line 172) <https://reviews.apache.org/r/55132/#comment232072> Please make sure you remove unnecessary white spaces. You can easily spot them here in the Review Board with checking the "View Diff" view where they show up with red highlighting. src/java/org/apache/sqoop/manager/Db2Manager.java (line 175) <https://reviews.apache.org/r/55132/#comment232057> I would recommend to use a named constant instead of "String" to make the code more clean and undertandable. src/java/org/apache/sqoop/manager/Db2Manager.java (line 219) <https://reviews.apache.org/r/55132/#comment232058> I would recommend to use a named constant instead of "String" to make the code more clean and undertandable. src/test/org/apache/sqoop/manager/db2/DB2XmlTypeImportManualTest.java (line 34) <https://reviews.apache.org/r/55132/#comment232059> Please avoid using wildcard imports based on the common Sqoop coding guidelines. src/test/org/apache/sqoop/manager/db2/DB2XmlTypeImportManualTest.java (lines 154 - 155) <https://reviews.apache.org/r/55132/#comment232062> Including the exception itself here will also include the stack trace, there is no need for calling the printStackTrace method separately. src/test/org/apache/sqoop/manager/db2/DB2XmlTypeImportManualTest.java (line 174) <https://reviews.apache.org/r/55132/#comment232064> It would be a little more useful to include the exception itself here not just the name of it as it would also include it's stack trace. src/test/org/apache/sqoop/manager/db2/DB2XmlTypeImportManualTest.java (lines 233 - 234) <https://reviews.apache.org/r/55132/#comment232070> Same here with the stack trace. src/test/org/apache/sqoop/manager/db2/DB2XmlTypeImportManualTest.java (lines 248 - 249) <https://reviews.apache.org/r/55132/#comment232071> Same here with the stack trace. - Boglarka Egyed On Jan. 3, 2017, 9:39 a.m., Ying Cao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55132/ > ----------------------------------------------------------- > > (Updated Jan. 3, 2017, 9:39 a.m.) > > > Review request for Sqoop, Abraham Elmahrek and Jarek Cecho. > > > Bugs: SQOOP-1904 > https://issues.apache.org/jira/browse/SQOOP-1904 > > > Repository: sqoop-trunk > > > Description > ------- > > SQOOP-1904: support for DB2 XML data type when importing to hdfs > > > Diffs > ----- > > src/java/org/apache/sqoop/manager/Db2Manager.java e39aa4c > src/test/com/cloudera/sqoop/ThirdPartyTests.java 7e10c68 > src/test/org/apache/sqoop/manager/db2/DB2XmlTypeImportManualTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/55132/diff/ > > > Testing > ------- > > UT passed by manually > > > Thanks, > > Ying Cao > >
