> Am 05.12.2016 um 22:44 schrieb Roger Riggs <[email protected]>:
> 
> 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 change the 
> actual setting, otherwise a new instance with the requested autoFlush is 
> returned.
Sounds good to me.  If there are no other suggestions I will use that one..

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

Right, even the Charset is actually available within the StreamEncoder 
implementation but not provided to the outside world.

Also the OutputStreamWriter is in wrapped in a BufferedWriter and would be 
needed to be extracted from there again in the first place. 
>> 
>>> -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 
>>>> <http://cr.openjdk.java.net/~reinhapa/reviews/8167648/webrev.00>
>>>> 
>>>> -Patrick
> 

Reply via email to