sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jarek Cecho" <jar...@apache.org>
Subject Re: Review Request 35556: SQOOP-1094: Add avro support to merge tool
Date Thu, 23 Jul 2015 16:16:16 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35556/#review92755
-----------------------------------------------------------


I have few nits:

1) Generally please nuke all trailing whitespaces. You can see them when applying the patch
with "git apply" command or review board will show red box at the end of the line.


src/java/org/apache/sqoop/mapreduce/MergeAvroMapper.java (line 37)
<https://reviews.apache.org/r/35556/#comment147000>

    Nit: Seems like copy&paste comment :)



src/java/org/apache/sqoop/mapreduce/MergeAvroMapper.java (line 52)
<https://reviews.apache.org/r/35556/#comment147001>

    Nit: whitespace



src/java/org/apache/sqoop/mapreduce/MergeJob.java (lines 179 - 198)
<https://reviews.apache.org/r/35556/#comment147002>

    Can we move this method to some AvroUtils [1]?
    
    1: https://github.com/apache/sqoop/blob/trunk/src/java/org/apache/sqoop/avro/AvroUtil.java


Jarcec

- Jarek Cecho


On July 23, 2015, 12:02 a.m., Yibing Shi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35556/
> -----------------------------------------------------------
> 
> (Updated July 23, 2015, 12:02 a.m.)
> 
> 
> Review request for Sqoop, Abraham Elmahrek and Jarek Cecho.
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> -------
> 
> SQOOP-1094: Add avro support to merge tool
> 
> 
> Diffs
> -----
> 
>   src/java/org/apache/sqoop/mapreduce/AvroJob.java bb4755c 
>   src/java/org/apache/sqoop/mapreduce/MergeAvroMapper.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/MergeAvroReducer.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/MergeJob.java 4e2a916 
>   src/java/org/apache/sqoop/mapreduce/MergeReducer.java cafff8a 
>   src/java/org/apache/sqoop/mapreduce/MergeReducerBase.java PRE-CREATION 
>   src/test/com/cloudera/sqoop/TestMerge.java 3821aa1 
> 
> Diff: https://reviews.apache.org/r/35556/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Yibing Shi
> 
>


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