From dev-return-14136-apmail-sqoop-dev-archive=sqoop.apache.org@sqoop.apache.org Mon Oct 20 18:31:46 2014 Return-Path: X-Original-To: apmail-sqoop-dev-archive@www.apache.org Delivered-To: apmail-sqoop-dev-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 69A7217D47 for ; Mon, 20 Oct 2014 18:31:46 +0000 (UTC) Received: (qmail 77345 invoked by uid 500); 20 Oct 2014 18:31:46 -0000 Delivered-To: apmail-sqoop-dev-archive@sqoop.apache.org Received: (qmail 77307 invoked by uid 500); 20 Oct 2014 18:31:46 -0000 Mailing-List: contact dev-help@sqoop.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@sqoop.apache.org Delivered-To: mailing list dev@sqoop.apache.org Received: (qmail 77294 invoked by uid 500); 20 Oct 2014 18:31:45 -0000 Delivered-To: apmail-incubator-sqoop-dev@incubator.apache.org Received: (qmail 77287 invoked by uid 99); 20 Oct 2014 18:31:45 -0000 Received: from reviews-vm.apache.org (HELO reviews.apache.org) (140.211.11.40) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 20 Oct 2014 18:31:45 +0000 Received: from reviews.apache.org (localhost [127.0.0.1]) by reviews.apache.org (Postfix) with ESMTP id 1CC8F1DB868; Mon, 20 Oct 2014 18:31:51 +0000 (UTC) Content-Type: multipart/alternative; boundary="===============4205237663935817002==" MIME-Version: 1.0 Subject: Re: Review Request 26941: SQOOP-1557: SQ_CONFIGURABLE ( for entities who own configs) From: "Veena Basavaraj" To: "Veena Basavaraj" , "Sqoop" , "Jarek Cecho" Date: Mon, 20 Oct 2014 18:31:51 -0000 Message-ID: <20141020183151.1283.51823@reviews.apache.org> X-ReviewBoard-URL: https://reviews.apache.org Auto-Submitted: auto-generated Sender: "Veena Basavaraj" X-ReviewGroup: Sqoop X-ReviewRequest-URL: https://reviews.apache.org/r/26941/ X-Sender: "Veena Basavaraj" References: <20141020182630.1282.31534@reviews.apache.org> In-Reply-To: <20141020182630.1282.31534@reviews.apache.org> Reply-To: "Veena Basavaraj" X-ReviewRequest-Repository: sqoop-sqoop2 --===============4205237663935817002== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit > 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 > > --===============4205237663935817002==--