----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8679/#review14815 -----------------------------------------------------------
Very few minor comments. branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java <https://reviews.apache.org/r/8679/#comment31518> To be removed branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java <https://reviews.apache.org/r/8679/#comment31517> Thanks for cleaning up. It is annoying to see objects passed to methods to be updated. Makes it difficult to understand what is happening in code. Can you use StringBuilder instead of StringBuffer as there is no thread safety involved. It is good to return strings instead of StringBuffer as it is better to keep return types immutable. But not a big deal and fine if you are trying to avoid one more object construction. branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java <https://reviews.apache.org/r/8679/#comment31519> Can you move this just before dependencyList.put(pullOrPush, depList);. Just for sake of readability branches/hcat-intre/core/src/main/java/org/apache/oozie/dependency/HCatURIHandler.java <https://reviews.apache.org/r/8679/#comment31521> ErrorCode.E1025, uri, e branches/hcat-intre/core/src/main/java/org/apache/oozie/jms/HCatMessageHandler.java <https://reviews.apache.org/r/8679/#comment31522> Earlier I meant removing mentioning thrift all together from code. i.e getting index of :// and doing a substring after that to remove the scheme. branches/hcat-intre/core/src/main/java/org/apache/oozie/dependency/FSURIHandler.java <https://reviews.apache.org/r/8679/#comment31520> extra space to be removed. - Rohini Palaniswamy On Dec. 21, 2012, 1:48 a.m., Virag Kothari wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8679/ > ----------------------------------------------------------- > > (Updated Dec. 21, 2012, 1:48 a.m.) > > > Review request for oozie. > > > Description > ------- > > https://issues.apache.org/jira/browse/OOZIE-1138 > > > This addresses bug OOZIE-1138. > https://issues.apache.org/jira/browse/OOZIE-1138 > > > Diffs > ----- > > branches/hcat-intre/core/src/main/java/org/apache/oozie/ErrorCode.java > 1424271 > > branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordActionUpdatePushMissingDependency.java > 1424271 > > branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java > 1424271 > > branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java > 1424271 > > branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordPushDependencyCheckXCommand.java > 1424271 > > branches/hcat-intre/core/src/main/java/org/apache/oozie/dependency/FSURIHandler.java > 1424271 > > branches/hcat-intre/core/src/main/java/org/apache/oozie/dependency/HCatURIHandler.java > 1424271 > > branches/hcat-intre/core/src/main/java/org/apache/oozie/dependency/URIHandler.java > 1424271 > > branches/hcat-intre/core/src/main/java/org/apache/oozie/jms/HCatMessageHandler.java > 1424271 > > branches/hcat-intre/core/src/main/java/org/apache/oozie/service/JMSAccessorService.java > 1424271 > > branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java > 1424271 > > branches/hcat-intre/core/src/main/java/org/apache/oozie/util/MappingRule.java > PRE-CREATION > > branches/hcat-intre/core/src/main/java/org/apache/oozie/util/PartitionWrapper.java > 1424271 > branches/hcat-intre/core/src/main/resources/oozie-default.xml 1424271 > > branches/hcat-intre/core/src/test/java/org/apache/oozie/service/TestJMSAccessorService.java > 1424271 > > branches/hcat-intre/core/src/test/java/org/apache/oozie/service/TestPartitionDependencyManagerService.java > 1424271 > > Diff: https://reviews.apache.org/r/8679/diff/ > > > Testing > ------- > > end to end test done and connections to jms server verified > > > Thanks, > > Virag Kothari > >
