Request for Review and Sponsor needed: JDK-8167648: java.io.PrintWriter should have PrintWriter((String|File), Charset) constructors (original) (raw)

Patrick Reinhart patrick at reini.net
Wed Nov 30 21:47:23 UTC 2016

Hi Roger,

Am 30.11.2016 um 20:01 schrieb Roger Riggs <Roger.Riggs at Oracle.com>:

Hi Patrick, I have reservations about trying to get this into JDK 9. Because it is a new API, it should have some bake time before feature freeze and it needs further review from the compatibility point of view and resources committed to create new JCK tests. Many folks are fully loaded also trying to hit feature freeze.

I did not expect that the first iteration is good enough. I took this issue, because it was marked for target 9. So I’m open for other views.

A few comments on the webrev:

- 359: The withAutoFlush javadoc should be more explicit about when a new vs the same PrintWriter is returned. The ‚activates‘ verb doesn't convey any sense about the instance that is returned.

359 * Activates {@code printf}, or {@code format} methods to automatically 360 * flush the output buffer after writing their data.

Would the following be better:

Returns the same instance if {@code autoFlush} does not change the actual setting, otherwise a new instance with changed behavior is returned

- 375: Can this use the new private constructor that will handle psOut.

Here I can not get hold on the encoding at this point or have I missed something here?

-320, etc. The @since should be 1 or 2 digits to match the version scheme

It seems, that I’m still not in the new version scheme ;-) - should be 9 - I will change that, the same for 343

- no tests for new PrintWriter(OutputStream , Charset)

I will also add that

- From the test file name 'FailngConstructors", its not clear that's the right place for the positive tests of the withAutoFlush methods.

I will move that out to a new Test as soon we are more clear about the other points

That's all I have time for at the moment,

Regards, Roger

On 11/29/2016 4:15 PM, Patrick Reinhart wrote: Does anyone sponsor this fix?

http://cr.openjdk.java.net/~reinhapa/reviews/8167648/webrev.00 -Patrick

More information about the core-libs-dev mailing list