Hi Roger,
Thank you for the feedback. I have addressed your comments and
updated the Webrev.
http://cr.openjdk.java.net/~alvdavi/webrevs/8235699/
Regards,
Clive Verghese
On 2/4/20, 12:34 PM, "core-libs-dev on behalf of Roger Riggs"
<core-libs-dev-boun...@openjdk.java.net on behalf of
roger.ri...@oracle.com> wrote:
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
>
>
>
>
