Hi Clive,
A few comments:
The copyrights for the new files need to follow the template in
<repo>make/template/gpl-header.
In particular, it needs to include Oracle as a copyright holder.
bug235699.java:
The @summary should be more informative; "it works" tells nothing about
the case being tested.
Please rename bug8235699.java to Bug8235699.java;
There are more initial upper case tests than lower and it would be good
to converge over time.
35: A comment saying that a AIOOBE should not occur would be helpful.
CalendarBuilderTest.java:
Should describe the condition that is being created.
27: "Test that CalendarBuilder.toString does not produce IOOBE"
Are all of the assignments necessary to cause the bug?
Remove any that are not; they are misleading. (onlysetting the year is
needed)
Thanks, Roger
On 1/2/20 4:18 PM, Verghese, Clive wrote:
Hi Alan,
Thanks for the feedback,
I have removed the @Author tag and updated the tests as per your recommendation.
Updated Webrev
http://cr.openjdk.java.net/~phh/8235699/webrev.04/
Regards,
Clive Verghsese
Regards,
Clive Verghese
On 1/2/20, 11:19 AM, "Volker Simonis" <simon...@amazon.de> wrote:
On 02.01.20 18:39, Alan Bateman wrote:
> On 02/01/2020 13:26, Volker Simonis wrote:
>> :
>> http://cr.openjdk.java.net/~simonis/webrevs/2020/8235699.02/
>>
>> Ready to push?
>>
> You shouldn't need to use core reflection here. Instead you can create
> the test so that it is compiled and runs as if part of the java.text
> package, e.g.
>
> @build java.base/java.text.CalendarBuilderToStringTest
> @main Driver
>
Thanks for the hint. I wasn't aware of this possibility.
I think Clive will rewrite the test.
> Do you really want the @author tag? We try to avoid them if possible
> because they are so hard to remove, even when code/tests are changed
> significantly.
No not really. It was just a part of the template I used :)
@Clive: please feel free to remove the author tag.
> -Alan
