----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8803/#review15210 -----------------------------------------------------------
Ship it! The changes look good to me. One minor comment that I had is that it would be helpful for a reviewer if you added some comments why the these assertions hold. I was able to understand the reason only after reading the loadSubmissions() method in DerbyTestCase. assertTrue(handler.inUseJob(1, getDerbyConnection())); assertFalse(handler.inUseJob(2, getDerbyConnection())); assertFalse(handler.inUseJob(3, getDerbyConnection())); assertFalse(handler.inUseJob(4, getDerbyConnection())); Adding a simple comment like "// See DerbyTestCase regarding why these assertions must hold" will be very helpful. I don't see many comments in test cases now. I guess we can improve it in the future. - Cheolsoo Park On Jan. 2, 2013, 6:57 p.m., Jarek Cecho wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8803/ > ----------------------------------------------------------- > > (Updated Jan. 2, 2013, 6:57 p.m.) > > > Review request for Sqoop. > > > Description > ------- > > I've added proposed functionality. > > > This addresses bug SQOOP-807. > https://issues.apache.org/jira/browse/SQOOP-807 > > > Diffs > ----- > > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java > c62730ccd077663c763ece7df0c044acd437ffcc > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestConnectionHandling.java > 4121be74364de33e0cae0aa1fb469a8c7d632922 > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestJobHandling.java > c0e7a313e887e1fbca244ca92f7cf8b8a1d44bd4 > > Diff: https://reviews.apache.org/r/8803/diff/ > > > Testing > ------- > > I've added unit test + one unit test for similar function with Connection > object. > > > Thanks, > > Jarek Cecho > >
