Re: RFR 8196298 Add null Reader and Writer with latest changes

[Roger Riggs]

Hi Patrick,

Committed;  Thanks for the initiative and the perseverance.

Roger

On 3/18/2018 6:41 AM, Patrick Reinhart wrote:

Hi Roger,

Seems there are no more comments...   - all shocked :-)

-Patrick

Re: RFR 8196298 Add null Reader and Writer with latest changes

[Patrick Reinhart]

Hi Roger,

Seems there are no more comments...   - all shocked :-)

-Patrick

Am 16.03.2018 um 16:21 schrieb Roger Riggs:
> Hi Patrick,
>
> Looks good,
>
> If there are no more comments, I can sponsor the commit.
>
> Thanks, Roger
>
>
> On 3/16/18 2:10 AM, Patrick Reinhart wrote:
>> Just coming back on my webrev [1]. Are there any more feedbacks
>> implementation wise to that latest version? If no I would need a
>> commit sponsor for this change as the CSR [2] is reviewed and closed.
>>
>> -Patrick
>>
>> [1] http://cr.openjdk.java.net/~reinhapa/reviews/8196298/webrev.02
>> [2] https://bugs.openjdk.java.net/browse/JDK-8196350
>>
>>
>>> Am 07.03.2018 um 11:41 schrieb Patrick Reinhart :
>>>
>>> I applied those changes here:
>>>
>>> http://cr.openjdk.java.net/~reinhapa/reviews/8196298/webrev.02
>>>
>>> -Patrick
>>>
>>>
 Am 06.03.2018 um 23:12 schrieb Brian Burkhalter
 :

 Yes, I think so and and also the parameter ‘csq' is allowed to be
 null so Objects.requireNonNull(csq) should be removed at lines 100
 and 107 as no NPE is specified for these methods [1, 2].

 Thanks,

 Brian

 [1]
 https://download.java.net/java/jdk10/docs/api/java/io/Writer.html#append(java.lang.CharSequence)
 [2]
 https://download.java.net/java/jdk10/docs/api/java/io/Writer.html#append(java.lang.CharSequence,int,int)

 On Mar 6, 2018, at 11:49 AM, Bernd Eckenfels
  wrote:

> Just a nit, Should append(CharSequence,int,int) also use
> checkFromIndexSize?
>

Re: RFR 8196298 Add null Reader and Writer with latest changes

[Roger Riggs]

Hi Patrick,

Looks good,

If there are no more comments, I can sponsor the commit.

Thanks, Roger


On 3/16/18 2:10 AM, Patrick Reinhart wrote:

Just coming back on my webrev [1]. Are there any more feedbacks implementation 
wise to that latest version? If no I would need a commit sponsor for this 
change as the CSR [2] is reviewed and closed.

-Patrick

[1] http://cr.openjdk.java.net/~reinhapa/reviews/8196298/webrev.02
[2] https://bugs.openjdk.java.net/browse/JDK-8196350



Am 07.03.2018 um 11:41 schrieb Patrick Reinhart :

I applied those changes here:

http://cr.openjdk.java.net/~reinhapa/reviews/8196298/webrev.02

-Patrick



Am 06.03.2018 um 23:12 schrieb Brian Burkhalter :

Yes, I think so and and also the parameter ‘csq' is allowed to be null so 
Objects.requireNonNull(csq) should be removed at lines 100 and 107 as no NPE is 
specified for these methods [1, 2].

Thanks,

Brian

[1] 
https://download.java.net/java/jdk10/docs/api/java/io/Writer.html#append(java.lang.CharSequence)
[2] 
https://download.java.net/java/jdk10/docs/api/java/io/Writer.html#append(java.lang.CharSequence,int,int)

On Mar 6, 2018, at 11:49 AM, Bernd Eckenfels  wrote:


Just a nit, Should append(CharSequence,int,int) also use checkFromIndexSize?

Re: RFR 8196298 Add null Reader and Writer with latest changes

[Patrick Reinhart]

I applied those changes here:

http://cr.openjdk.java.net/~reinhapa/reviews/8196298/webrev.02

-Patrick


> Am 06.03.2018 um 23:12 schrieb Brian Burkhalter :
> 
> Yes, I think so and and also the parameter ‘csq' is allowed to be null so 
> Objects.requireNonNull(csq) should be removed at lines 100 and 107 as no NPE 
> is specified for these methods [1, 2].
> 
> Thanks,
> 
> Brian
> 
> [1] 
> https://download.java.net/java/jdk10/docs/api/java/io/Writer.html#append(java.lang.CharSequence)
> [2] 
> https://download.java.net/java/jdk10/docs/api/java/io/Writer.html#append(java.lang.CharSequence,int,int)
> 
> On Mar 6, 2018, at 11:49 AM, Bernd Eckenfels  wrote:
> 
>> Just a nit, Should append(CharSequence,int,int) also use checkFromIndexSize?
> 

Re: RFR 8196298 Add null Reader and Writer with latest changes

[Brian Burkhalter]

Yes, I think so and and also the parameter ‘csq' is allowed to be null so 
Objects.requireNonNull(csq) should be removed at lines 100 and 107 as no NPE is 
specified for these methods [1, 2].

Thanks,

Brian

[1] 
https://download.java.net/java/jdk10/docs/api/java/io/Writer.html#append(java.lang.CharSequence)
[2] 
https://download.java.net/java/jdk10/docs/api/java/io/Writer.html#append(java.lang.CharSequence,int,int)

On Mar 6, 2018, at 11:49 AM, Bernd Eckenfels  wrote:

> Just a nit, Should append(CharSequence,int,int) also use checkFromIndexSize?

Re: RFR 8196298 Add null Reader and Writer with latest changes

[Bernd Eckenfels]

Just a nit, Should append(CharSequence,int,int) also use checkFromIndexSize?

Greetings
Bernd
-- 
http://bernd.eckenfels.net

Von: Alan Bateman
Gesendet: Dienstag, 6. März 2018 20:35
An: Patrick Reinhart; core-libs-dev
Betreff: Re: RFR 8196298 Add null Reader and Writer with latest changes

On 05/03/2018 21:45, Patrick Reinhart wrote:
> I tried to incorporate all feedback I received so far and updated the webrev 
> accordingly:
>
> http://cr.openjdk.java.net/~reinhapa/reviews/8196298/webrev.01 
> <http://cr.openjdk.java.net/~reinhapa/reviews/8196298/webrev.01>
>
I think this looks good now. I've added myself as a reviewer on the CSR 
so you should be able to finalize it.

-Alan

Re: RFR 8196298 Add null Reader and Writer with latest changes

[Patrick Reinhart]

Updated the CSR API with webrev's content and finalized it..

-Patrick

Am 06.03.2018 um 17:35 schrieb Alan Bateman:
> On 05/03/2018 21:45, Patrick Reinhart wrote:
>> I tried to incorporate all feedback I received so far and updated the
>> webrev accordingly:
>>
>> http://cr.openjdk.java.net/~reinhapa/reviews/8196298/webrev.01
>> 
>>
> I think this looks good now. I've added myself as a reviewer on the
> CSR so you should be able to finalize it.
>
> -Alan

Re: RFR 8196298 Add null Reader and Writer with latest changes

[Alan Bateman]

On 05/03/2018 21:45, Patrick Reinhart wrote:

I tried to incorporate all feedback I received so far and updated the webrev 
accordingly:

http://cr.openjdk.java.net/~reinhapa/reviews/8196298/webrev.01 


I think this looks good now. I've added myself as a reviewer on the CSR 
so you should be able to finalize it.


-Alan