> On Aug. 26, 2014, 3:15 p.m., Abraham Elmahrek wrote: > > common/src/test/java/org/apache/sqoop/model/TestFormUtils.java, lines 30-34 > > <https://reviews.apache.org/r/25074/diff/1/?file=669587#file669587line30> > > > > Is this move necessary?
I probably need to set the import order. using the formatter reordered. I wil revert it back. > On Aug. 26, 2014, 3:15 p.m., Abraham Elmahrek wrote: > > common/src/main/java/org/apache/sqoop/model/FormUtils.java, line 553 > > <https://reviews.apache.org/r/25074/diff/1/?file=669585#file669585line553> > > > > void instead of boolean? second rev already has it > On Aug. 26, 2014, 3:15 p.m., Abraham Elmahrek wrote: > > common/src/main/java/org/apache/sqoop/model/FormUtils.java, lines 202-212 > > <https://reviews.apache.org/r/25074/diff/1/?file=669585#file669585line202> > > > > This works! I'm wondering if we can simply have a lookup table > > somewhere! this means we need to persisit both. Not sure if adding a new field for custom is worth. If we go that route, then the field name should be just a blob supporting customized names and will more work. > On Aug. 26, 2014, 3:15 p.m., Abraham Elmahrek wrote: > > common/src/main/java/org/apache/sqoop/model/FormUtils.java, lines 302-305 > > <https://reviews.apache.org/r/25074/diff/1/?file=669585#file669585line302> > > > > These don't need to be on different lines. It the eclipse formmater on the wiki that I used. > On Aug. 26, 2014, 3:15 p.m., Abraham Elmahrek wrote: > > common/src/main/java/org/apache/sqoop/model/FormUtils.java, lines 323-326 > > <https://reviews.apache.org/r/25074/diff/1/?file=669585#file669585line323> > > > > This doesn't seem to need to be on different lines. same as above - Veena ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25074/#review51592 ----------------------------------------------------------- On Aug. 26, 2014, 2:40 p.m., Veena Basavaraj wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25074/ > ----------------------------------------------------------- > > (Updated Aug. 26, 2014, 2:40 p.m.) > > > Review request for Sqoop, Abraham Elmahrek, Gwen Shapira, and Jarek Cecho. > > > Bugs: SQOOP-1436 > https://issues.apache.org/jira/browse/SQOOP-1436 > > > Repository: sqoop-sqoop2 > > > Description > ------- > > https://issues.apache.org/jira/browse/SQOOP-1436 > As discussed in the SQOOP-1436, this feature gives the ability to specifiy a > custom form name. It is optional, if given it is persisted in the metadata > repo. Methods that convert the form fields to JSON have also been modified to > read the custom form name attribute. basic validation for uniqueness, length > and pattern have been added > > Existing unit tests modified to support the custom form name attribute > unit tests added for validation cases > > Note: there are formatting changes since I used the mandated eclipse > formatter.xml in the IDE > > > Diffs > ----- > > common/src/main/java/org/apache/sqoop/model/Form.java 4321582 > common/src/main/java/org/apache/sqoop/model/FormUtils.java 27db8af > common/src/main/java/org/apache/sqoop/model/ModelError.java 1f466fe > common/src/test/java/org/apache/sqoop/model/TestFormUtils.java 08dfa7b > > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ConnectionConfiguration.java > 6061600 > > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ExportJobConfiguration.java > f2b2d65 > > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ImportJobConfiguration.java > f3c1d13 > > core/src/main/java/org/apache/sqoop/framework/configuration/ConnectionConfiguration.java > 830606a > > core/src/main/java/org/apache/sqoop/framework/configuration/ExportJobConfiguration.java > 6665429 > > core/src/main/java/org/apache/sqoop/framework/configuration/ImportJobConfiguration.java > 2a35eb9 > > Diff: https://reviews.apache.org/r/25074/diff/ > > > Testing > ------- > > mvn test and integration tests pass. > > > Thanks, > > Veena Basavaraj > >
