Thank you, Vyom Regards, Chris
> On 6 Aug 2018, at 2:02 PM, vyom tewari <vyom.tew...@oracle.com> wrote: > > Hi Chris, > > Latest webrev looks good to me. > > Thanks, > > Vyom > > > On Friday 03 August 2018 02:46 PM, Chris Yin wrote: >> Hi, Vyom >> >> Thank a lot for your review and comments, inline and update new webrev as >> below >> >> http://cr.openjdk.java.net/~xyin/8208279/webrev.02/ >> >>> On 3 Aug 2018, at 3:45 PM, vyom tewari <vyom.tew...@oracle.com> wrote: >>> >>> Hi Chris, >>> >>> Overall looks good to me, couple of minor comment. >>> >>> 1-> ProviderUrlGen.java, Line:100 Exception e can be replaced with specific >>> exceptions. >> Fixed, thanks >> >>> 2-> In couple of places we are calling "removeFromEnvironment" on context >>> and spec says it will return "null" if the env is not set. I see that we >>> are setting the required env in "initContext" so it will never be "null" >>> in our tests, but i suggest you to put just one liner comment that "val" >>> can not be null. This is just to improve code readability, please feel free >>> to ignore, if you think it is not worth enough. >> Sure, added one line comment before each “removeFromEnvironment” call as you >> suggested >> >> Thanks, >> Chris >> >>> Thanks, >>> >>> Vyom >>> >>> >>> On Monday 30 July 2018 08:41 AM, Chris Yin wrote: >>>> Please find the new webrev as below, it addressed some similar issues >>>> which mentioned in review comments from another thread RFR 8200151, thanks >>>> >>>> webrev: http://cr.openjdk.java.net/~xyin/8208279/webrev.01/ >>>> <http://cr.openjdk.java.net/~xyin/8208279/webrev.01/> >>>> >>>> Regards, >>>> Chris >>>> >>>>> On 26 Jul 2018, at 3:37 PM, Chris Yin <xu.y....@oracle.com> wrote: >>>>> >>>>> Please review the changes to add another 8 JNDI tests to >>>>> com/sun/jndi/dns/EnvTests/ in OpenJDK, thanks >>>>> >>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8208279 >>>>> <https://bugs.openjdk.java.net/browse/JDK-8208279> >>>>> webrev: http://cr.openjdk.java.net/~xyin/8208279/webrev.00/ >>>>> <http://cr.openjdk.java.net/~xyin/8208279/webrev.00/> >>>>> >>>>> Regards, >>>>> Chris >
