----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4472/#review6486 -----------------------------------------------------------
Ship it! it looks good. the only thing I'd ask to be done with the testcase is the following: Instead loading the conf file as a resource from the classloader and having the included file as 'src/main/test/resources/...', both files should be copied to the testcase directory (obtained via XTestCase.getTestDir() ) and the include should be relative with no subdirs. then the stream of the main conf should be read from there. the reason for this is that while maven runs the testcases from trunk/core/, IDEs run it from trunk/ (or the current dir) and the testcase will fail from within the IDE - Alejandro On 2012-03-23 23:41:10, Virag Kothari wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4472/ > ----------------------------------------------------------- > > (Updated 2012-03-23 23:41:10) > > > Review request for oozie. > > > Summary > ------- > > Adding support for Xinclude in xml which is similar to what Hadoop has > > > This addresses bug oozie-780. > https://issues.apache.org/jira/browse/oozie-780 > > > Diffs > ----- > > trunk/core/src/main/java/org/apache/oozie/util/XConfiguration.java 1304647 > trunk/core/src/test/java/org/apache/oozie/util/TestXConfiguration.java > 1304647 > trunk/core/src/test/resources/test-hadoop-config.xml 1304647 > trunk/core/src/test/resources/test-oozie-default.xml 1304647 > > Diff: https://reviews.apache.org/r/4472/diff > > > Testing > ------- > > > Thanks, > > Virag > >
