On 28/08/2013 22:10, Henry Jen wrote:
Hi,
Please review the webrev at
http://cr.openjdk.java.net/~henryjen/ccc/8017513/1/webrev/
Based on the feedback/discussion from last time, the EG decided to
weaken AutoCloseable contract(see RFR 8022176[1]), and have Stream
extend AutoCloseable.
A quick briefing of the webrev,
- Remove CloseableStream and DelegatingStream, which was previous used
to implement close of resource
- BaseStream extends AutoCloseable, thus all Stream implementation will
implements close()
- onClose() method allows to chain up close handlers which are invoked
by close()
- Change use of CloseableStream to Stream, mainly java.nio.file.Files
and various tests.
The specdiff is also available at
http://cr.openjdk.java.net/~henryjen/ccc/8017513/1/specdiff/overview-summary.html
Cheers,
Henry
[1]
http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-August/020236.html
In Files then you've expanded the wildcard on the imports but left
java.nio.file.attribute.*; (this seems a long list, I think one or two
may be javadoc references only).
I think I mentioned this previously but in the Files.list/walk/etc.
methods where you close the resource (on error|runtimeexception) then
it's probably best to catch the IOException and add it as a suppressed
exception.
Minor nits in Files but all the <p> usages after a space before the
wording. Also can you combine L123-124 on the same line so that the
formatting is locally consistent.
StreamCloseTest.java has the GPL+CP copyright, I assume you'll replace
that before pushing.
Otherwise looks good to me.
-Alan.
