----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4196/#review5642 -----------------------------------------------------------
Looks good most of my comments are a request for some unit tests especially where stuff has been refactored and there is new code. Some of the stuff wasn't a change as the code was just reformatted. trunk/pge/src/main/java/org/apache/oodt/cas/pge/PGETaskInstance.java <https://reviews.apache.org/r/4196/#comment12279> Why is this gone? I see the new one that runs all. Mark this with deprecated? It's protected so only internally used just wondered. trunk/pge/src/main/java/org/apache/oodt/cas/pge/PGETaskInstance.java <https://reviews.apache.org/r/4196/#comment12280> Not really a change just a reformatting. trunk/pge/src/main/java/org/apache/oodt/cas/pge/PGETaskInstance.java <https://reviews.apache.org/r/4196/#comment12281> no change just reformatting trunk/pge/src/main/java/org/apache/oodt/cas/pge/PGETaskInstance.java <https://reviews.apache.org/r/4196/#comment12276> Is this one of the keys you were saying is still needed in the patch? trunk/pge/src/main/java/org/apache/oodt/cas/pge/PGETaskInstance.java <https://reviews.apache.org/r/4196/#comment12282> Where has this functionality moved to? trunk/pge/src/main/java/org/apache/oodt/cas/pge/config/FileBasedPgeConfigBuilder.java <https://reviews.apache.org/r/4196/#comment12285> This appears to be a refactoring make sure the unit tests written for the XMLFilePGEConfigBuilder cover the code here. trunk/pge/src/main/java/org/apache/oodt/cas/pge/config/XmlFilePgeConfigBuilder.java <https://reviews.apache.org/r/4196/#comment12284> Lots of changes needs unit tests trunk/pge/src/main/java/org/apache/oodt/cas/pge/metadata/PGETaskMetKeys.java <https://reviews.apache.org/r/4196/#comment12278> yay constants. trunk/pge/src/main/java/org/apache/oodt/cas/pge/metadata/PgeMetadata.java <https://reviews.apache.org/r/4196/#comment12277> Yay unused code removed. trunk/pge/src/main/java/org/apache/oodt/cas/pge/staging/FileManagerFileStager.java <https://reviews.apache.org/r/4196/#comment12283> Needs unit tests. Also make sure there is coverage into FileStager. - Paul On 2012-03-06 08:20:36, brian Foster wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4196/ > ----------------------------------------------------------- > > (Updated 2012-03-06 08:20:36) > > > Review request for oodt, Chris Mattmann and Paul Ramirez. > > > Summary > ------- > > This includes the changes from wengine-branch cas-pge... still needs to add > backwards compatibility for MetKeys (i.e. PCS_* etc) > > > Diffs > ----- > > trunk/pge/src/main/java/org/apache/oodt/cas/pge/PGETaskInstance.java > 1297147 > > trunk/pge/src/main/java/org/apache/oodt/cas/pge/config/FileBasedPgeConfigBuilder.java > PRE-CREATION > trunk/pge/src/main/java/org/apache/oodt/cas/pge/config/FileStagingInfo.java > PRE-CREATION > trunk/pge/src/main/java/org/apache/oodt/cas/pge/config/PgeConfig.java > 1297147 > > trunk/pge/src/main/java/org/apache/oodt/cas/pge/config/PgeConfigBuilder.java > 1297147 > > trunk/pge/src/main/java/org/apache/oodt/cas/pge/config/PgeConfigMetKeys.java > 1297147 > > trunk/pge/src/main/java/org/apache/oodt/cas/pge/config/XmlFilePgeConfigBuilder.java > 1297147 > > trunk/pge/src/main/java/org/apache/oodt/cas/pge/metadata/PGETaskMetKeys.java > PRE-CREATION > trunk/pge/src/main/java/org/apache/oodt/cas/pge/metadata/PgeMetadata.java > 1297147 > > trunk/pge/src/main/java/org/apache/oodt/cas/pge/staging/FileManagerFileStager.java > PRE-CREATION > trunk/pge/src/main/java/org/apache/oodt/cas/pge/staging/FileStager.java > PRE-CREATION > > trunk/pge/src/main/java/org/apache/oodt/cas/pge/writers/PcsMetFileWriter.java > 1297147 > > Diff: https://reviews.apache.org/r/4196/diff > > > Testing > ------- > > > Thanks, > > brian > >
