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 > > > >