Codereview request for 7096080: UTF8 update and new CESU-8 charset

Xueming Shen xueming.shen at oracle.com
Mon Nov 7 21:28:53 UTC 2011


Hi Ulf,

Good catch. It was the leftover of my first drift, in which 
CESU.de/encoder is the
subclass of the UTF8 de/encoder.

webrev has been updated accordingly.

-Sherman

On 11/07/2011 12:57 PM, Ulf Zibis wrote:
> Hi Sherman,
>
> 2 nits again:
> protected static final class Encoder extends CharsetEncoder
> - If final, protected doesn't make sense, as it can't be subclassed. 
> (ideally this should be a compiler error)
> - Where is it used? Otherwise it could be private.
>
> -Ulf
>
>
> Am 07.11.2011 21:30, schrieb Xueming Shen:
>> Thanks Alan,
>>
>> The webrev has been updated accordingly to address your comments (the
>> commented out isMalformed2 has been removed, copyright updated, bugid
>> added...)
>>
>> And thanks for Ulf for the detailed review.
>>
>> -Sherman
>>
>> On 10/28/2011 09:14 AM, Alan Bateman wrote:
>>> On 28/09/2011 20:18, Xueming Shen wrote:
>>>> Hi,
>>>>
>>>> [I combined the proposed charge for #7082884, in which no one 
>>>> appears to be
>>>> interested:-) into this one]
>>>> :
>>>>
>>>> http://cr.openjdk.java.net/~sherman/7096080/webrev/ 
>>>> <http://cr.openjdk.java.net/%7Esherman/7096080/webrev/>
>>> I don't know if you are still looking for a reviewer for this (seems 
>>> like Ulf has gone through this in detail, thanks Ulf).
>>>
>>> Overall it looks fine to me. Minor comment is that in UTF_8.java 
>>> then maybe isMalformed2 should be removed completely, maybe move 
>>> some of the comment in the decode methods. Another minor nits is 
>>> that the date on CESU_8.java is 2000-2010 where I assume it should 
>>> be 2011. In Errors.java then it might be better to just remove L196 
>>> as it might confuse future maintainers. I would also suggest adding 
>>> the bugID to the list of bugs in the tests too as someone these 
>>> references are useful.
>>>
>>> -Alan.
>>
>>




More information about the core-libs-dev mailing list