Hi Patrick,

On 11/30/2016 4:47 PM, Patrick Reinhart wrote:
...
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

my $.02 version

Returns the same instance if {@code autoFlush} does not changethe actual 
setting, otherwise a new instance with*the requested autoFlush *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?
True, not easily, it is available as a String from OutputStreamWriter.getEncoding() but it would suffer from
having to lookup it by name again.


-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
ok

Thanks, Roger

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

Reply via email to