> On Oct. 13, 2014, 2:20 p.m., Jarek Cecho wrote: > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/DerbyTestCase.java, > > lines 205-206 > > <https://reviews.apache.org/r/26657/diff/1/?file=719686#file719686line205> > > > > The reason why we have the code duplicated here, is that if somebody > > will unintentionally change the code in DerbyRepositoryHandler, then all > > tests will start breaking and he will realize that the change has been > > invalid as it requires an upgrade path. Hence unless we will somehow > > accomplish this goal differently, I would prefer to keep this particular > > code duplication. > > Veena Basavaraj wrote: > There are other ways to handle such things, than duplicating code, I am > happy to create a ticket for this and fix it the right way, instead of > debating. I really dont like how copy paste is considered a solution for > things.
BTW, when I made changed for SQOOP-1498, I did not do the upgrade logic the right way, neither the tests broke nor did I find any documentation on the right way to upgrade. So I am not sure if you still think the copy paste magic i helping guard a developer to not break functionality. - Veena ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26657/#review56443 ----------------------------------------------------------- On Oct. 13, 2014, 1:03 p.m., Veena Basavaraj wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26657/ > ----------------------------------------------------------- > > (Updated Oct. 13, 2014, 1:03 p.m.) > > > Review request for Sqoop. > > > Repository: sqoop-sqoop2 > > > Description > ------- > > fix the upgrade logic for 1498 changes. > While fixing the upgrade logic, added comments on how the upgrade logic works. > -renamed test methods to reflect what they are upgrading and what they are > infact testing. > -renamed the internals to exactly mean "schema" upgrades, since we only > allow/ do scheme upgrades the repository > -Also updated the names on the repository api to clearly state the difference > between upgrade and update operations on connector / driver. > > > Diffs > ----- > > core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java 3ade247 > core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java > 97de893 > core/src/main/java/org/apache/sqoop/repository/Repository.java 95c7a4d > core/src/main/java/org/apache/sqoop/repository/RepositoryManager.java > c2f8505 > core/src/test/java/org/apache/sqoop/repository/TestJdbcRepository.java > e6e4760 > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoConstants.java > 74e41df > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java > 0f0f7c4 > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java > 10a7b1a > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaConstants.java > cf6e657 > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java > 56ea147 > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/DerbyTestCase.java > 9316687 > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestConnectorHandling.java > fc95222 > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestDriverHandling.java > d597bd8 > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestInputTypes.java > 260c2a9 > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestInternals.java > 0eb9df4 > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestJobHandling.java > 01a05b2 > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestLinkHandling.java > bbfe5bb > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestRespositorySchemaUpgrade.java > PRE-CREATION > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestSubmissionHandling.java > 8402d8c > > Diff: https://reviews.apache.org/r/26657/diff/ > > > Testing > ------- > > yes > > > Thanks, > > Veena Basavaraj > >
