oodt-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "brian Foster" <holen...@juno.com>
Subject Re: Review Request: Updates from branch cas-pge
Date Fri, 09 Mar 2012 08:16:42 GMT


> On 2012-03-06 15:23:53, Paul Ramirez wrote:
> > 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.

Hey paul thanks for the comments, also should have noted that i'm gonna use this as like the
master patch... will make a visual for me so i can create smaller incremental patches (if
possible... lol)... lots of bug fixes and improvements were added to wengine's cas-pge...
also cas-pge needs unit tests... currently only has one test which does nothing! lol... also
needs cas-cli integration as well, so xml validation actions and the like can be written and
possibly actions like workflow emulation mode actions so cas-pge can be run stand-alone but
act like it is part of a workflow... this will probably take me the next century to accomplish
but i'm gonna start chopping away at it :)


> On 2012-03-06 15:23:53, Paul Ramirez wrote:
> > trunk/pge/src/main/java/org/apache/oodt/cas/pge/PGETaskInstance.java, lines 146-149
> > <https://reviews.apache.org/r/4196/diff/1/?file=88545#file88545line146>
> >
> >     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.

not gone, signature changed to:

   protected void runPropertyAdder(ConfigFilePropertyAdder propAdder, PgeConfig pgeConfig,
PgeMetadata pgeMetadata) {


> On 2012-03-06 15:23:53, Paul Ramirez wrote:
> > trunk/pge/src/main/java/org/apache/oodt/cas/pge/PGETaskInstance.java, lines 238-239
> > <https://reviews.apache.org/r/4196/diff/1/?file=88545#file88545line238>
> >
> >     Not really a change just a reformatting.

The class lacked formatting... updated it to 3 space formatting


> On 2012-03-06 15:23:53, Paul Ramirez wrote:
> > trunk/pge/src/main/java/org/apache/oodt/cas/pge/PGETaskInstance.java, line 282
> > <https://reviews.apache.org/r/4196/diff/1/?file=88545#file88545line282>
> >
> >     no change just reformatting

The class lacked formatting... updated it to 3 space formatting


> On 2012-03-06 15:23:53, Paul Ramirez wrote:
> > trunk/pge/src/main/java/org/apache/oodt/cas/pge/PGETaskInstance.java, lines 464-465
> > <https://reviews.apache.org/r/4196/diff/1/?file=88545#file88545line464>
> >
> >     Is this one of the keys you were saying is still needed in the patch?
> >

yup


> On 2012-03-06 15:23:53, Paul Ramirez wrote:
> > trunk/pge/src/main/java/org/apache/oodt/cas/pge/PGETaskInstance.java, lines 548-550
> > <https://reviews.apache.org/r/4196/diff/1/?file=88545#file88545line548>
> >
> >     Where has this functionality moved to?

Still gotta work this back in... wengine's TaskInstance would update a tasks metadata for
you... PgeMetadata existed in wengine as ControlMetadata


> On 2012-03-06 15:23:53, Paul Ramirez wrote:
> > trunk/pge/src/main/java/org/apache/oodt/cas/pge/config/FileBasedPgeConfigBuilder.java,
line 41
> > <https://reviews.apache.org/r/4196/diff/1/?file=88546#file88546line41>
> >
> >     This appears to be a refactoring make sure the unit tests written for the XMLFilePGEConfigBuilder
cover the code here.

will do


> On 2012-03-06 15:23:53, Paul Ramirez wrote:
> > trunk/pge/src/main/java/org/apache/oodt/cas/pge/config/XmlFilePgeConfigBuilder.java,
line 69
> > <https://reviews.apache.org/r/4196/diff/1/?file=88551#file88551line69>
> >
> >     Lots of changes needs unit tests

will do!


> On 2012-03-06 15:23:53, Paul Ramirez wrote:
> > trunk/pge/src/main/java/org/apache/oodt/cas/pge/metadata/PGETaskMetKeys.java, line
3
> > <https://reviews.apache.org/r/4196/diff/1/?file=88552#file88552line3>
> >
> >     yay constants.

acknowledged 


> On 2012-03-06 15:23:53, Paul Ramirez wrote:
> > trunk/pge/src/main/java/org/apache/oodt/cas/pge/metadata/PgeMetadata.java, line
241
> > <https://reviews.apache.org/r/4196/diff/1/?file=88553#file88553line241>
> >
> >     Yay unused code removed.

acknowledged


> On 2012-03-06 15:23:53, Paul Ramirez wrote:
> > trunk/pge/src/main/java/org/apache/oodt/cas/pge/staging/FileManagerFileStager.java,
line 1
> > <https://reviews.apache.org/r/4196/diff/1/?file=88554#file88554line1>
> >
> >     Needs unit tests. Also make sure there is coverage into FileStager.

will do!


- brian


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4196/#review5642
-----------------------------------------------------------


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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message