You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@oodt.apache.org by brian Foster <ho...@juno.com> on 2012/03/22 22:27:55 UTC
Review Request: CrawlerBeansPropHandler doesn't set list properties for
Spring PropertyOverrideConfigurer correctly
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4454/
-----------------------------------------------------------
Review request for oodt, Chris Mattmann, Ricky Nguyen, Paul Ramirez, and Thomas Bennett.
Summary
-------
It currently was setting lists keys to: value1,value2,value3,...
This patch will instead change it to:
key[0] = value1
key[1] = value2
This addresses bug OODT-428.
https://issues.apache.org/jira/browse/OODT-428
Diffs
-----
trunk/crawler/src/main/java/org/apache/oodt/cas/crawl/cli/option/handler/CrawlerBeansPropHandler.java 1304041
Diff: https://reviews.apache.org/r/4454/diff
Testing
-------
still need a unit-test for this... will write when i get the time
Thanks,
brian
Re: Review Request: CrawlerBeansPropHandler doesn't set list properties for
Spring PropertyOverrideConfigurer correctly
Posted by Thomas Bennett <lm...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4454/#review6279
-----------------------------------------------------------
Ship it!
I've tested it. Looks good to me!
trunk/crawler/src/main/java/org/apache/oodt/cas/crawl/cli/option/handler/CrawlerBeansPropHandler.java
<https://reviews.apache.org/r/4454/#comment13622>
May need to test for boolean optionsInstances, i.e. where optionInstance.getValues().size() == 0.
- Thomas
On 2012-03-22 21:27:55, brian Foster wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4454/
> -----------------------------------------------------------
>
> (Updated 2012-03-22 21:27:55)
>
>
> Review request for oodt, Chris Mattmann, Ricky Nguyen, Paul Ramirez, and Thomas Bennett.
>
>
> Summary
> -------
>
> It currently was setting lists keys to: value1,value2,value3,...
>
> This patch will instead change it to:
> key[0] = value1
> key[1] = value2
>
>
> This addresses bug OODT-428.
> https://issues.apache.org/jira/browse/OODT-428
>
>
> Diffs
> -----
>
> trunk/crawler/src/main/java/org/apache/oodt/cas/crawl/cli/option/handler/CrawlerBeansPropHandler.java 1304041
>
> Diff: https://reviews.apache.org/r/4454/diff
>
>
> Testing
> -------
>
> still need a unit-test for this... will write when i get the time
>
>
> Thanks,
>
> brian
>
>
Re: Review Request: CrawlerBeansPropHandler doesn't set list properties for
Spring PropertyOverrideConfigurer correctly
Posted by brian Foster <ho...@juno.com>.
> On 2012-03-26 14:22:31, Chris Mattmann wrote:
> > trunk/crawler/src/main/java/org/apache/oodt/cas/crawl/cli/option/handler/CrawlerBeansPropHandler.java, line 60
> > <https://reviews.apache.org/r/4454/diff/1/?file=94689#file94689line60>
> >
> > Agree with Tom here, do we need a check on if getValues() == null too?
i think what i'll do here is check if it is empty and then throw an Exception explicitly so it is clear that a no arg option it wrong here... boolean options should have a static arg specifying either true or false value.
- brian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4454/#review6347
-----------------------------------------------------------
On 2012-03-22 21:27:55, brian Foster wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4454/
> -----------------------------------------------------------
>
> (Updated 2012-03-22 21:27:55)
>
>
> Review request for oodt, Chris Mattmann, Ricky Nguyen, Paul Ramirez, and Thomas Bennett.
>
>
> Summary
> -------
>
> It currently was setting lists keys to: value1,value2,value3,...
>
> This patch will instead change it to:
> key[0] = value1
> key[1] = value2
>
>
> This addresses bug OODT-428.
> https://issues.apache.org/jira/browse/OODT-428
>
>
> Diffs
> -----
>
> trunk/crawler/src/main/java/org/apache/oodt/cas/crawl/cli/option/handler/CrawlerBeansPropHandler.java 1304041
>
> Diff: https://reviews.apache.org/r/4454/diff
>
>
> Testing
> -------
>
> still need a unit-test for this... will write when i get the time
>
>
> Thanks,
>
> brian
>
>
Re: Review Request: CrawlerBeansPropHandler doesn't set list properties for
Spring PropertyOverrideConfigurer correctly
Posted by Chris Mattmann <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4454/#review6347
-----------------------------------------------------------
LGTM, see my comment about the null?
trunk/crawler/src/main/java/org/apache/oodt/cas/crawl/cli/option/handler/CrawlerBeansPropHandler.java
<https://reviews.apache.org/r/4454/#comment13772>
Agree with Tom here, do we need a check on if getValues() == null too?
- Chris
On 2012-03-22 21:27:55, brian Foster wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4454/
> -----------------------------------------------------------
>
> (Updated 2012-03-22 21:27:55)
>
>
> Review request for oodt, Chris Mattmann, Ricky Nguyen, Paul Ramirez, and Thomas Bennett.
>
>
> Summary
> -------
>
> It currently was setting lists keys to: value1,value2,value3,...
>
> This patch will instead change it to:
> key[0] = value1
> key[1] = value2
>
>
> This addresses bug OODT-428.
> https://issues.apache.org/jira/browse/OODT-428
>
>
> Diffs
> -----
>
> trunk/crawler/src/main/java/org/apache/oodt/cas/crawl/cli/option/handler/CrawlerBeansPropHandler.java 1304041
>
> Diff: https://reviews.apache.org/r/4454/diff
>
>
> Testing
> -------
>
> still need a unit-test for this... will write when i get the time
>
>
> Thanks,
>
> brian
>
>
Re: Review Request: CrawlerBeansPropHandler doesn't set list properties for
Spring PropertyOverrideConfigurer correctly
Posted by Chris Mattmann <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4454/#review6380
-----------------------------------------------------------
Ship it!
Sounds good LGTM.
- Chris
On 2012-03-26 20:55:22, brian Foster wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4454/
> -----------------------------------------------------------
>
> (Updated 2012-03-26 20:55:22)
>
>
> Review request for oodt, Chris Mattmann, Ricky Nguyen, Paul Ramirez, and Thomas Bennett.
>
>
> Summary
> -------
>
> It currently was setting lists keys to: value1,value2,value3,...
>
> This patch will instead change it to:
> key[0] = value1
> key[1] = value2
>
>
> This addresses bug OODT-428.
> https://issues.apache.org/jira/browse/OODT-428
>
>
> Diffs
> -----
>
> trunk/crawler/src/main/java/org/apache/oodt/cas/crawl/cli/option/handler/CrawlerBeansPropHandler.java 1304041
> trunk/crawler/src/test/org/apache/oodt/cas/crawl/cli/option/handler/TestCrawlerBeansPropHandler.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/4454/diff
>
>
> Testing
> -------
>
> Wrote unit-test
>
>
> Thanks,
>
> brian
>
>
Re: Review Request: CrawlerBeansPropHandler doesn't set list properties for
Spring PropertyOverrideConfigurer correctly
Posted by Chris Mattmann <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4454/#review6386
-----------------------------------------------------------
Ship it!
LGTM.
- Chris
On 2012-03-26 20:55:22, brian Foster wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4454/
> -----------------------------------------------------------
>
> (Updated 2012-03-26 20:55:22)
>
>
> Review request for oodt, Chris Mattmann, Ricky Nguyen, Paul Ramirez, and Thomas Bennett.
>
>
> Summary
> -------
>
> It currently was setting lists keys to: value1,value2,value3,...
>
> This patch will instead change it to:
> key[0] = value1
> key[1] = value2
>
>
> This addresses bug OODT-428.
> https://issues.apache.org/jira/browse/OODT-428
>
>
> Diffs
> -----
>
> trunk/crawler/src/main/java/org/apache/oodt/cas/crawl/cli/option/handler/CrawlerBeansPropHandler.java 1304041
> trunk/crawler/src/test/org/apache/oodt/cas/crawl/cli/option/handler/TestCrawlerBeansPropHandler.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/4454/diff
>
>
> Testing
> -------
>
> Wrote unit-test
>
>
> Thanks,
>
> brian
>
>
Re: Review Request: CrawlerBeansPropHandler doesn't set list properties for
Spring PropertyOverrideConfigurer correctly
Posted by brian Foster <ho...@juno.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4454/
-----------------------------------------------------------
(Updated 2012-03-26 20:55:22.461160)
Review request for oodt, Chris Mattmann, Ricky Nguyen, Paul Ramirez, and Thomas Bennett.
Changes
-------
- For case optionInstance.getValues().isEmpty() now throws RuntimeException
- Added Unit-tests
Summary
-------
It currently was setting lists keys to: value1,value2,value3,...
This patch will instead change it to:
key[0] = value1
key[1] = value2
This addresses bug OODT-428.
https://issues.apache.org/jira/browse/OODT-428
Diffs (updated)
-----
trunk/crawler/src/main/java/org/apache/oodt/cas/crawl/cli/option/handler/CrawlerBeansPropHandler.java 1304041
trunk/crawler/src/test/org/apache/oodt/cas/crawl/cli/option/handler/TestCrawlerBeansPropHandler.java PRE-CREATION
Diff: https://reviews.apache.org/r/4454/diff
Testing (updated)
-------
Wrote unit-test
Thanks,
brian