RFR (JDK10) 8183743: Umbrella: add overloads that take a Charset parameter
Roger Riggs
Roger.Riggs at Oracle.com
Tue Dec 12 16:47:55 UTC 2017
Hi Joe,
Looks good; +1
Regards, Roger
On 12/11/2017 3:04 PM, Joe Wang wrote:
>
> On 12/11/17, 11:18 AM, Roger Riggs wrote:
>> Hi Joe,
>>
>> The new tests using testng look good, a few comments:
>>
>> - BAOS/EncodingTest.java
>> 70: The message gets printed if the condition is not true; so
>> "results do not might" might read better
>> (also, there is Assert.assertEquals(str1, str2, msg) - that
>> compares objects with .equals())
>> A message in nice, though in many cases the testng generated
>> exception/message is sufficient
>> especially for equals.
>
> Assert.assertEquals it is! Removed other debug printout.
>
>>
>> - PrintStream/EncodingTest.java
>> 92: out should not be null; its probably a test bug; the test
>> should be removed
>
> Done.
>>
>> - PrintWriter/EncodingTest.java
>> 93: out should not be null; its probably a test bug; the test
>> should be removed
>
> Done.
>
> Updated webrev:
> http://cr.openjdk.java.net/~joehw/jdk10/8183743/webrev/index.html
>
> Thanks,
> Joe
>
>>
>> Thanks, Roger
>>
>>
>> On 12/8/2017 7:29 PM, Joe Wang wrote:
>>> Thanks Roger, Alan for the comments!
>>>
>>> I've updated the webrev accordingly. Here's the current webrev:
>>> http://cr.openjdk.java.net/~joehw/jdk10/8183743/webrev/index.html
>>>
>>> -Joe
>>>
>>> On 12/4/17, 7:52 PM, Joe Wang wrote:
>>>>
>>>>
>>>> On 12/3/17, 11:34 AM, Alan Bateman wrote:
>>>>>
>>>>>
>>>>> On 01/12/2017 20:35, Joe Wang wrote:
>>>>>> :
>>>>>>
>>>>>>>
>>>>>>> I think URLDecoder.decode methods should have @throws
>>>>>>> IllegalArgumentException. I realize it's implementation specific
>>>>>>> as to whether IAE is thrown with bad input but given that the RI
>>>>>>> does throw IAE then consumers of the API should be prepared for
>>>>>>> it. The @implNote can stay and probably should be copied into
>>>>>>> the legacy decode method too.
>>>>>>
>>>>>> I added @throws IAE. On a 2nd thought, would that give no
>>>>>> flexibility to alternative impls as the general (class) spec had
>>>>>> given? With this addition, it becomes a requirement.
>>>>> If the @throws description starts with "if the implementation ..."
>>>>> then it should be clear that it is optional.
>>>>
>>>> Thanks Alan. I made the change accordingly.
>>>> http://cr.openjdk.java.net/~joehw/jdk10/8183743/webrev/index.html
>>>>
>>>> -Joe
>>>>
>>>>>
>>>>> -Alan
>>
More information about the core-libs-dev
mailing list