Re: Review Request: BufferedReader needs to be closed after use in loadScript method in SqlScript class

2013-03-23 Thread Chris Mattmann

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

Ship it!


Ship It!

- Chris Mattmann


On March 23, 2013, 3:03 p.m., Ross Laidlaw wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9965/
> ---
> 
> (Updated March 23, 2013, 3:03 p.m.)
> 
> 
> Review request for oodt and Chris Mattmann.
> 
> 
> Description
> ---
> 
> The loadScript method uses a BufferedReader resource but doesn't close it 
> after use.  Since the method declares 'throws IOException', we can use 
> try...finally and close the reader in the finally block.  Additionally, it 
> looks like the method deals with files and strings only so we can remove the 
> 'throws SQLException' from the method statement.
> 
> 
> This addresses bug OODT-576.
> https://issues.apache.org/jira/browse/OODT-576
> 
> 
> Diffs
> -
> 
>   
> /trunk/commons/src/main/java/org/apache/oodt/commons/database/SqlScript.java 
> 1457043 
> 
> Diff: https://reviews.apache.org/r/9965/diff/
> 
> 
> Testing
> ---
> 
> I ran the unit tests for the commons package.  I also ran the 
> TestWorkflowDataSourceRepository tests from the workflow package, as these 
> tests use a setUp method that makes a call to the loadScript method in 
> SqlScript.
> 
> 
> Thanks,
> 
> Ross Laidlaw
> 
>



Re: Review Request: BufferedReader needs to be closed after use in loadScript method in SqlScript class

2013-03-23 Thread Ross Laidlaw

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

(Updated March 23, 2013, 3:03 p.m.)


Review request for oodt and Chris Mattmann.


Description (updated)
---

The loadScript method uses a BufferedReader resource but doesn't close it after 
use.  Since the method declares 'throws IOException', we can use try...finally 
and close the reader in the finally block.  Additionally, it looks like the 
method deals with files and strings only so we can remove the 'throws 
SQLException' from the method statement.


This addresses bug OODT-576.
https://issues.apache.org/jira/browse/OODT-576


Diffs
-

  /trunk/commons/src/main/java/org/apache/oodt/commons/database/SqlScript.java 
1457043 

Diff: https://reviews.apache.org/r/9965/diff/


Testing
---

I ran the unit tests for the commons package.  I also ran the 
TestWorkflowDataSourceRepository tests from the workflow package, as these 
tests use a setUp method that makes a call to the loadScript method in 
SqlScript.


Thanks,

Ross Laidlaw



Review Request: BufferedReader needs to be closed after use in loadScript method in SqlScript class

2013-03-15 Thread Ross Laidlaw

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

Review request for oodt and Chris Mattmann.


Description
---

The loadScript method uses a BufferedReader resource but doesn't close it after 
use.  Since the method declares 'throws IOException', we can use try...finally 
and close the reader in the finally block.  Additionally, it looks like the 
method deals with files and strings only (no SQL stuff), so we can remove the 
'throws SQLException' from the method statement.


This addresses bug OODT-576.
https://issues.apache.org/jira/browse/OODT-576


Diffs
-

  /trunk/commons/src/main/java/org/apache/oodt/commons/database/SqlScript.java 
1457043 

Diff: https://reviews.apache.org/r/9965/diff/


Testing
---

I ran the unit tests for the commons package.  I also ran the 
TestWorkflowDataSourceRepository tests from the workflow package, as these 
tests use a setUp method that makes a call to the loadScript method in 
SqlScript.


Thanks,

Ross Laidlaw