sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Boglarka Egyed <egyedb...@gmail.com>
Subject Re: Review Request 55132: support for DB2 XML data type when importing to hdfs
Date Sun, 08 Jan 2017 12:57:09 GMT

-----------------------------------------------------------
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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message