----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26941/#review57652 -----------------------------------------------------------
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java <https://reviews.apache.org/r/26941/#comment98453> Super nit: Can we also put the semicolon on it's own line? :) My goal here is to have ability to add new exception code by just adding a new line without need to touch other lines which will make it easier for "git cherrypick" or "git blame". common/src/main/java/org/apache/sqoop/model/MConnector.java <https://reviews.apache.org/r/26941/#comment98454> Seems as still the same invalid import? :) common/src/main/java/org/apache/sqoop/model/MDriver.java <https://reviews.apache.org/r/26941/#comment98455> Thank you for cleaning this one! core/src/main/java/org/apache/sqoop/driver/Driver.java <https://reviews.apache.org/r/26941/#comment98456> Do you think that it would make sense to change this call to Driver.getClass().getSimpleName() so that we don't have to change it if at some point in the future we decides to change the package or class name? core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java <https://reviews.apache.org/r/26941/#comment98457> I'm thinking if it would be claner to call here mDriver.getUniqueName() instead of going to the Driver class directly? It's more a nitpick at this point, but this methods assumes that the mDriver will be derived from Driver. Which right now will definitely be the case, but I'm wondering whether we want to have such assumption here. core/src/main/java/org/apache/sqoop/repository/Repository.java <https://reviews.apache.org/r/26941/#comment98452> Seems as unused import? repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java <https://reviews.apache.org/r/26941/#comment98458> Can we keep the comment here so that it's obvious what we're doing here? repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java <https://reviews.apache.org/r/26941/#comment98459> Nit: Trailing whitespace repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java <https://reviews.apache.org/r/26941/#comment98460> Nit: Trailing whitespace repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java <https://reviews.apache.org/r/26941/#comment98461> Nit: Trailing whitespace repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/DerbyTestCase.java <https://reviews.apache.org/r/26941/#comment98462> Nit: Trailing whitespace Jarcec - Jarek Cecho On Oct. 21, 2014, 7:19 p.m., Veena Basavaraj wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26941/ > ----------------------------------------------------------- > > (Updated Oct. 21, 2014, 7:19 p.m.) > > > Review request for Sqoop. > > > Repository: sqoop-sqoop2 > > > Description > ------- > > see JIRA for details > > there is whitespace, that will be addressed once the reviews for the > functionality > > > Diffs > ----- > > common/src/main/java/org/apache/sqoop/model/MConnector.java 174d0b9 > common/src/main/java/org/apache/sqoop/model/MDriver.java 4241a31 > core/src/main/java/org/apache/sqoop/driver/Driver.java 46a16ac > core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java 476830d > core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java > 4c5229f > core/src/main/java/org/apache/sqoop/repository/Repository.java 8f78052 > core/src/test/java/org/apache/sqoop/repository/TestJdbcRepository.java > ff9e0c3 > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoConstants.java > 40dcc49 > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java > 3e4a4a9 > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java > aa58850 > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaConstants.java > 59773e1 > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java > 44ec2e3 > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/DerbyTestCase.java > 366e4ee > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestConnectorHandling.java > 68a173b > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestDriverHandling.java > bbf721f > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestJobHandling.java > a15bda9 > tools/src/main/java/org/apache/sqoop/tools/tool/RepositoryLoadTool.java > 8cf9cf1 > > Diff: https://reviews.apache.org/r/26941/diff/ > > > Testing > ------- > > yes > > > Thanks, > > Veena Basavaraj > >
