This is an automatically generated e-mail. To reply, visit:
Few comments:
core/src/main/java/org/apache/sqoop/core/ConfigurationConstants.java (line 87)

    Nit: Might I suggest to rename this variable to org.apache.sqoop.classpath.job or something. So that it's in the same "prefix" as the property CLASSPATH.

core/src/main/java/org/apache/sqoop/core/SqoopConfiguration.java (lines 280 - 310)

    I somehow feel that the job of parsing the configuration property shouldn't be here but rather in the JobManager class.
    The SqoopConfiguration is a general class that in general doesn't know about semantics of underlaying properties (there are few exceptions though) and the semantics is given in the code that is actually using the configuration propertly. Good example of that is the property org.apache.sqoop.classpath.extra that also contains extra classpath entires, but yet we're not parsing them in this class.

core/src/main/java/org/apache/sqoop/driver/configuration/JarConfig.java (lines 26 - 30)

    You will need to create entries in driver-config.properties:
    Might I suggest to port this tescase to Driver as well?
    The test case would uncover this missing piece.

core/src/main/java/org/apache/sqoop/driver/configuration/JarConfig.java (line 28)

    Super nit: Please put space between the "//" and "A"

dist/pom.xml (line 194)

    This change doesn't seem to be relevant to this patch, so let's nuke it?

- Jarek Cecho
On Oct. 6, 2015, 7:48 p.m., Abraham Fine wrote: