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


Most of my comments are inline with Gwen. 

In my very sweet words this RB has atleast 50% of the exact same things Derby 
code has. Lets not create more tech debt with this c&p. Pull a lot of things 
out into common classes or even Utils.

Even the test methods can be shared, kindly spend time extracting it out to a 
utils code

Also, maintaining the sql scripts in code is a pain especially with upgrade, is 
there a way to avoid this going forward. If we need ideas, a simple brain dead 
solution is to have versioned sql scripts that are invoked from code.

- Veena Basavaraj


On Oct. 20, 2014, 11:47 a.m., Abraham Elmahrek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26948/
> -----------------------------------------------------------
> 
> (Updated Oct. 20, 2014, 11:47 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-1523
>     https://issues.apache.org/jira/browse/SQOOP-1523
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> commit 0702bddfa69f63f56bfc6909dfe1b3e4f3219c6a
> Author: Abraham Elmahrek <[email protected]>
> Date:   Tue Oct 14 12:05:53 2014 -0700
> 
>     Sqoop2: Postgresql repository
> 
> :100644 100644 5a8e026... 2c5c972... M  
> core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java
> :100644 100644 f25a29f... 5be6ad9... M  pom.xml
> :100644 100644 e3345c4... 8b308c9... M  repository/pom.xml
> :000000 100644 0000000... 19f67d4... A  
> repository/repository-postgresql/pom.xml
> :000000 100644 0000000... cdeed35... A  
> repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java
> :000000 100644 0000000... 872683e... A  
> repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java
> :000000 100644 0000000... 93352a6... A  
> repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java
> :000000 100644 0000000... d1ff898... A  
> repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java
> :000000 100644 0000000... d70dd86... A  
> repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java
> :000000 100644 0000000... 31cc6dc... A  
> repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java
> :000000 100644 0000000... a8f4242... A  
> repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestConnectorHandling.java
> :000000 100644 0000000... 44ffced... A  
> repository/repository-postgresql/src/test/resources/log4j.properties
> :100644 100644 67baaa5... 8ecdd01... M  server/pom.xml
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java 
> 5a8e026 
>   pom.xml f25a29f 
>   repository/pom.xml e3345c4 
>   repository/repository-postgresql/pom.xml PRE-CREATION 
>   
> repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java
>  PRE-CREATION 
>   
> repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java
>  PRE-CREATION 
>   
> repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java
>  PRE-CREATION 
>   
> repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java
>  PRE-CREATION 
>   
> repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java
>  PRE-CREATION 
>   
> repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java
>  PRE-CREATION 
>   
> repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestConnectorHandling.java
>  PRE-CREATION 
>   repository/repository-postgresql/src/test/resources/log4j.properties 
> PRE-CREATION 
>   server/pom.xml 67baaa5 
> 
> Diff: https://reviews.apache.org/r/26948/diff/
> 
> 
> Testing
> -------
> 
> Can start Sqoop2 server.
> 
> 
> Thanks,
> 
> Abraham Elmahrek
> 
>

Reply via email to