Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-02 Thread Xuelei Fan
> Please review these JNDI changes. > Bug detail: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7072353 > webrev: http://cr.openjdk.java.net/~mduigou/7072353/0/webrev/ Thanks for your effort to make JNDI free of compile-warning. The work is hard, I appreciate it. 1. I noticed the copyright d

Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-02 Thread Alan Bateman
Xuelei Fan wrote: : 1. I noticed the copyright date of a few files are unchanged, please update them before you push the changes. This has come up a few times but I don't think it is strictly required. Kelly or one of the release engineers run a script over the forest periodically to fix up

Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-02 Thread Xuelei Fan
On Aug 3, 2011, at 12:11 AM, Alan Bateman wrote: > Xuelei Fan wrote: >> : >> 1. I noticed the copyright date of a few files are unchanged, please >> update them before you push the changes. >> > This has come up a few times but I don't think it is strictly required. Kelly > or one of the rel

Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-02 Thread Alexandre Boulgakov
Thanks for reviewing! See my responses inline. I'll wait on sending another webrev until I've received the rest of your comments. -Sasha On 8/2/2011 2:19 AM, Xuelei Fan wrote: Please review these JNDI changes. Bug detail: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7072353 webrev: htt

Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-02 Thread Alexandre Boulgakov
Thanks for reviewing! Please see my responses inline. I'll wait on sending another webrev until I've received the rest of your comments. -Sasha On 8/2/2011 2:19 AM, Xuelei Fan wrote: Please review these JNDI changes. Bug detail: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7072353 webr

Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-02 Thread Xuelei Fan
com/sun/jndi/toolkit/dir/SearchFilter.java 451 for (NamingEnumeration ve = attr.getAll(); 452 ve.hasMore(); 453) { The update is OK. But the coding style looks uncomfortable. Would you mind change it to use for-each style? . javax/naming/directory/BasicAtt

Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-02 Thread Dr Andrew John Hughes
On 17:11 Tue 02 Aug , Alan Bateman wrote: > Xuelei Fan wrote: > > : > > 1. I noticed the copyright date of a few files are unchanged, please > > update them before you push the changes. > > > This has come up a few times but I don't think it is strictly required. > Kelly or one of the relea

Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-03 Thread David Holmes
Alexandre Boulgakov said the following on 08/03/11 04:44: On 8/2/2011 2:19 AM, Xuelei Fan wrote: 3017 Vector temp = (Vector)extractURLs(res.errorMessage); You may not need the conversion any more, the return value of extractURLs() has been updated to 2564 private static Vector extr

Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-03 Thread Alexandre Boulgakov
Please see my responses inline. Thanks! -Sasha On 8/2/2011 9:13 PM, Xuelei Fan wrote: . com/sun/jndi/toolkit/dir/SearchFilter.java 451 for (NamingEnumeration ve = attr.getAll(); 452 ve.hasMore(); 453) { The update is OK. But the coding style looks un

Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-03 Thread Alexandre Boulgakov
Users of Iterable expect to call Iterable.iterator() multiple times, receiving a fresh iterator pointing to the beginning of the Iterable each time. This would not be possible to do with an Enumeration wrapper since Enumerations are read-once. I don't know if this is a strong enough reason not

Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-03 Thread Xuelei Fan
On 8/4/2011 2:03 AM, Alexandre Boulgakov wrote: > Please see my responses inline. > > Thanks! > -Sasha > > On 8/2/2011 9:13 PM, Xuelei Fan wrote: >> . com/sun/jndi/toolkit/dir/SearchFilter.java >> 451 for (NamingEnumeration ve = attr.getAll(); >> 452 ve.hasMore(); >> 4

Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-04 Thread Neil Richards
On Wed, 2011-08-03 at 11:03 -0700, Alexandre Boulgakov wrote: > Please see my responses inline. > > Thanks! > -Sasha > > On 8/2/2011 9:13 PM, Xuelei Fan wrote: > > . com/sun/jndi/toolkit/dir/SearchFilter.java > > 451 for (NamingEnumeration ve = attr.getAll(); > > 452 ve.

Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-04 Thread Colin Decker
One better way to handle this in Java 8 would be to have a utility method that takes a Supplier> SAM argument (with a no-arg method that returns an Enumeration) and returns an Iterable that gets a new Enumeration from the supplier each time iterator() is called. It could then be used with method re