> On Sept. 23, 2015, 4:04 p.m., Ajay Yadava wrote: > > common/src/test/java/org/apache/falcon/entity/EntityUtilTest.java, line 414 > > <https://reviews.apache.org/r/38669/diff/1/?file=1082229#file1082229line414> > > > > nit: fully qualified name shouldn't be required I believe.
Yep. Thats the cluster that is in the import. Modified it and a few other methods with similar usage. > On Sept. 23, 2015, 4:04 p.m., Ajay Yadava wrote: > > common/src/test/java/org/apache/falcon/entity/EntityUtilTest.java, line 440 > > <https://reviews.apache.org/r/38669/diff/1/?file=1082229#file1082229line440> > > > > nit: wouldn't separating these two scenarios in two different test > > cases make tests more granular and independent of each other? I added it in the same method as it required the same setup (creation of cluster and process) as there aren't a whole lot of negative testcases. Didn't want that the setup to repeat (just adds time to UT). > On Sept. 23, 2015, 4:04 p.m., Ajay Yadava wrote: > > common/src/test/java/org/apache/falcon/entity/EntityUtilTest.java, line 431 > > <https://reviews.apache.org/r/38669/diff/1/?file=1082229#file1082229line431> > > > > nit : Now that you have created a constant for the separator shouldn't > > we be using that here instead of hardcoding? Required me to increase the access level of the constant. Addressed it, but, somehow code looks more verbose :-) - Pallavi ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38669/#review100201 ----------------------------------------------------------- On Sept. 23, 2015, 8:23 a.m., Pallavi Rao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38669/ > ----------------------------------------------------------- > > (Updated Sept. 23, 2015, 8:23 a.m.) > > > Review request for Falcon. > > > Bugs: FALCON-1483 > https://issues.apache.org/jira/browse/FALCON-1483 > > > Repository: falcon-git > > > Description > ------- > > Utility method that the native scheduler requires. > > This is linked to https://reviews.apache.org/r/35724/. All review comments > there have been incorporated in the patch. > > > Diffs > ----- > > common/src/main/java/org/apache/falcon/entity/EntityUtil.java ad41674 > common/src/test/java/org/apache/falcon/entity/EntityUtilTest.java d022bae > > Diff: https://reviews.apache.org/r/38669/diff/ > > > Testing > ------- > > UT added. Manual testing done. > > > Thanks, > > Pallavi Rao > >
