On Jan 26, 2015, at 3:52 AM, Alan Bateman <[email protected]> wrote:

> On 23/01/2015 19:17, Brian Burkhalter wrote:
>> Here’s an updated patch including a (perhaps overkill) test:
>> 
>> http://cr.openjdk.java.net/~bpb/6880737/webrev.01/
>> 

Here’s another updated patch:

http://cr.openjdk.java.net/~bpb/6880737/webrev.02/

> Thanks, the null check is fine. The webrev still has the javadoc change, it 
> looks okay but not needed.

I removed the javadoc change.

> The test needs to be moved into a FileLock sub-directory to keep the current 
> organization consistent.

Done. I had been unsure of adding a directory for just one test.

> As it's become a unit test for FileLock then I think it would be a bit 
> clearer as a TestNG test but not strictly required.

Left as-is.

> One thing that would be good is to switch the test to using FileChannel.open 
> and to use try-with-resources. The catching of Exception can probably be 
> dropped too, the test will fail if un-documented exceptions are throw.

Both done.

Thanks,

Brian

Reply via email to