> On Jan. 13, 2015, 8:34 a.m., Veena Basavaraj wrote:
> > connector/connector-sdk/src/main/java/org/apache/sqoop/connector/matcher/MatcherError.java,
> >  line 24
> > <https://reviews.apache.org/r/29849/diff/3/?file=819035#file819035line24>
> >
> >     pleae clean up 0000 if it is not used
> >     
> >     I am not sure why we need the 0001 checks since MatcherFactory 
> > instantiates these matchers after doing the right check

Let me add some more details on why I say the 0001 check is not required.
see the code in Matcher constructor, it is really confusing to have some code 
in MatcherFactroy and some code in Matcher Constructor, so if both are empty, 
then it is always set to some bytearray, so there is no need to do the extra 
check, it would have been nice if all this logic was in one place in 
MatcherFactroy.

    if (fromSchema.isEmpty() && toSchema.isEmpty()) {
      this.fromSchema = ByteArraySchema.getInstance();
      this.toSchema = ByteArraySchema.getInstance();


- Veena


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


On Jan. 13, 2015, 8:13 a.m., Qian Xu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29849/
> -----------------------------------------------------------
> 
> (Updated Jan. 13, 2015, 8:13 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-1988
>     https://issues.apache.org/jira/browse/SQOOP-1988
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> see jira
> 
> 
> Diffs
> -----
> 
>   
> connector/connector-sdk/src/main/java/org/apache/sqoop/connector/matcher/LocationMatcher.java
>  d92723e 
>   
> connector/connector-sdk/src/main/java/org/apache/sqoop/connector/matcher/Matcher.java
>  36ac5a5 
>   
> connector/connector-sdk/src/main/java/org/apache/sqoop/connector/matcher/MatcherError.java
>  577b091 
>   
> connector/connector-sdk/src/main/java/org/apache/sqoop/connector/matcher/NameMatcher.java
>  7cbc39f 
>   
> connector/connector-sdk/src/test/java/org/apache/sqoop/connector/matcher/TestLocationMatcher.java
>  624fa7b 
>   
> connector/connector-sdk/src/test/java/org/apache/sqoop/connector/matcher/TestNameMatcher.java
>  76ff0da 
> 
> Diff: https://reviews.apache.org/r/29849/diff/
> 
> 
> Testing
> -------
> 
> Existing tests passed 
> 
> 
> Thanks,
> 
> Qian Xu
> 
>

Reply via email to