Hi Attila, lower-case thumbs up, with one remark in addition to Sundar's. In BeansLinkerTest, related to the comment "No assertion for the setter; we just expect it to silently succeed" - if it's expected to succeed, shouldn't success be tested by verifying the set value?
Best, Michael > Am 11.01.2016 um 05:03 schrieb Sundararajan Athijegannathan > <sundararajan.athijegannat...@oracle.com>: > > * Spec. changes look good. > > * Review comments on code changes: > > Minor: BeansLinker.java - whitespace missing after "if"if(collectionType != > CollectionType.MAP && isFixedKey) > and few other "if" statements too. > > +1 > > PS. Not sure if the samples under $nashorn/samples have any dependency on > (older) BeansLinker behaviour. > > -Sundar > > On 1/6/2016 5:08 PM, Attila Szegedi wrote: >> Excellent, thanks for preparing it. It looks good to me. >> >> Attila. >> >>> On Jan 6, 2016, at 12:28 PM, Sundararajan Athijegannathan >>> <sundararajan.athijegannat...@oracle.com> wrote: >>> >>> Hi, >>> >>> specdiff for this API change is here -> >>> http://cr.openjdk.java.net/~sundar/8144919/dynalink_specdiff/overview-summary.html >>> >>> Thanks, >>> -Sundar >>> >>> On 12/24/2015 2:01 AM, Attila Szegedi wrote: >>>> Please review JDK-8144919 "Implement missing member handler for >>>> BeansLinker" at <http://cr.openjdk.java.net/~attila/8144919/webrev.jdk9> >>>> for <https://bugs.openjdk.java.net/browse/JDK-8144919> >>>> >>>> This change dependes on JDK-8144917, a review request for which >>>> immediately preceded this one. I only plan to commit the two together >>>> after they have been both reviewed. >>>> >>>> Note that I need a CCC review for this, as it touches the Dynalink public >>>> API (details in the JIRA issue). As such, even if I get 2 reviews on this, >>>> I will not be committing either this or 8144917 until CCC approval is >>>> obtained. >>>> >>>> Thanks, >>>> Attila. > -- <http://www.oracle.com/> Dr. Michael Haupt | Principal Member of Technical Staff Phone: +49 331 200 7277 | Fax: +49 331 200 7561 Oracle Java Platform Group | LangTools Team | Nashorn Oracle Deutschland B.V. & Co. KG, Schiffbauergasse 14 | 14467 Potsdam, Germany <http://www.oracle.com/commitment> Oracle is committed to developing practices and products that help protect the environment
