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

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 <non-null>, 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