> On July 22, 2013, 3:54 p.m., Jarek Cecho wrote: > > common/src/main/java/org/apache/sqoop/model/MFramework.java, line 37 > > <https://reviews.apache.org/r/12774/diff/1/?file=323718#file323718line37> > > > > Is there a reason for this variable to be package private? I would > > suggest to have it private if not. > > Raghav Gautam wrote: > version is a field that makes sense even for the MConnector (child class > of MFramework), that is why I did not make it private.
Thank you for the explanation Raghav, the explanation make sense to me. On July 22, 2013, 3:54 p.m., Raghav Gautam wrote: > > Jarcec > > Raghav Gautam wrote: > Uploading new patch. Thank you! - Jarek ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12774/#review23599 ----------------------------------------------------------- On July 22, 2013, 8:03 p.m., Raghav Gautam wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/12774/ > ----------------------------------------------------------- > > (Updated July 22, 2013, 8:03 p.m.) > > > Review request for Sqoop. > > > Bugs: SQOOP-1075 > https://issues.apache.org/jira/browse/SQOOP-1075 > > > Repository: sqoop-sqoop2 > > > Description > ------- > > A summary of changes: > 1. MFramework: added fields, getter & setter method for version. Made changes > to other methods to take version into account. > 2. MConnector: since it extends MFramework, can use his version field and > methods > 3. FrameworkBean, FormSerialization: encode/decode version field to json > 4. DerbyRepositoryHandler, DerbyRepoError: added private methods to > persist/fetch framework version; update framework version when > registering/updating framework > 5. TestFrameworkHandling: added test - the test checks for current version of > framework, changes it to lower version updates the framework and checks the > version of framework once again > 6. TestSqoopClient, TestUtils, TestMFramework, FrameworkManager, > DerbyTestCase: added version parameter to the MFramework constructor call. > > > Diffs > ----- > > common/src/main/java/org/apache/sqoop/common/VersionInfo.java dcf522f > common/src/main/java/org/apache/sqoop/json/FrameworkBean.java ad4753b > common/src/main/java/org/apache/sqoop/json/util/FormSerialization.java > 98768d6 > common/src/main/java/org/apache/sqoop/model/MConnector.java 1c2c422 > common/src/main/java/org/apache/sqoop/model/MFramework.java 694f022 > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoConstants.java > 607b8d5 > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java > aeb7533 > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java > f717abf > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/DerbyTestCase.java > 677b0be > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestFrameworkHandling.java > 66611d4 > > Diff: https://reviews.apache.org/r/12774/diff/ > > > Testing > ------- > > I have added new tests. > All the unit tests are passing. > > > Thanks, > > Raghav Gautam > >
