> On Oct. 20, 2014, 11:26 a.m., Jarek Cecho wrote: > > Overall I like the changes. I do have couple of high level notes: > > > > 1) Please do remove all the trailing whitespaces. I've mark some of them. > > Most of the IDEs do have option to automatically remove trailing whispaces > > on file save, so you might want to enable that option :) > > > > 2) It seems that this patch also incorporates quite a lot of changes into > > the query names and pretty much refactores the Query class. I don't mind > > doing those changes, but they should be covered by a separate JIRA as they > > are unrelated to "rename SQ_CONFIGURABLE".
WS yes, I dont usually give it much thought until the end of the review, I do have it enable din IDE, While I was doing the upgrade it was really hard for me to follow things, when people just add things without much documentation. If you want another RB, I will spend time on it, splitting this up, but I dont want to scarifice code readability for some superficial deadline, so I will split it up. - Veena ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26941/#review57377 ----------------------------------------------------------- On Oct. 20, 2014, 11:27 a.m., Veena Basavaraj wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26941/ > ----------------------------------------------------------- > > (Updated Oct. 20, 2014, 11:27 a.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/MConfigurableType.java > PRE-CREATION > common/src/main/java/org/apache/sqoop/model/MConnector.java 2f42191 > common/src/main/java/org/apache/sqoop/model/MDriver.java 685439e > core/src/main/java/org/apache/sqoop/driver/Driver.java f1b45bb > > 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 > c888910 > > 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 > > Diff: https://reviews.apache.org/r/26941/diff/ > > > Testing > ------- > > yes > > > Thanks, > > Veena Basavaraj > >
