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