Re: RFR 8191173 : (cl) Clarify or remove "for delegation" in ClassLoader spec

2017-11-23 Thread mandy chung



On 11/21/17 12:41 PM, Brent Christian wrote:

Hi,

Please review my change to this small bit of ClassLoader spec that can 
be tidied up, as noticed by Martin.


Issue:
https://bugs.openjdk.java.net/browse/JDK-8191173
Webrev:
http://cr.openjdk.java.net/~bchristi/8191173/webrev.00/

In java.lang.ClassLoader these methods:
  getParent()
  getPlatformClassLoader()
  getSystemClassLoader()
all state that the returned classloader is, "for delegation."

For getParent() this makes sense to mention.  But it seems unnecessary 
for the other two methods, which are static, and designed to always 
return the indicated classloader.  The getSystemClassLoader() docs go 
on to immediately mention that the system classloader is the default 
delegation parent.


Looks good.

As Alan points out, the parent class loader is only used for delegation 
when that class loader follows the delegation model.  The getParent 
method as well as the constructors could be made clear but I would leave 
it as is.


Mandy


Re: RFR 8191173 : (cl) Clarify or remove "for delegation" in ClassLoader spec

2017-11-21 Thread Alan Bateman



On 21/11/2017 20:41, Brent Christian wrote:

Hi,

Please review my change to this small bit of ClassLoader spec that can 
be tidied up, as noticed by Martin.


Issue:
https://bugs.openjdk.java.net/browse/JDK-8191173
Webrev:
http://cr.openjdk.java.net/~bchristi/8191173/webrev.00/
This looks okay. One could argue that getParent should be changed too as 
there is never any guarantee that delegation will be to the parent but 
what you have is fine.


-Alan


Re: RFR 8191173 : (cl) Clarify or remove "for delegation" in ClassLoader spec

2017-11-21 Thread David Holmes

Seems fine.

Thanks,
David

On 22/11/2017 6:41 AM, Brent Christian wrote:

Hi,

Please review my change to this small bit of ClassLoader spec that can 
be tidied up, as noticed by Martin.


Issue:
https://bugs.openjdk.java.net/browse/JDK-8191173
Webrev:
http://cr.openjdk.java.net/~bchristi/8191173/webrev.00/

In java.lang.ClassLoader these methods:
   getParent()
   getPlatformClassLoader()
   getSystemClassLoader()
all state that the returned classloader is, "for delegation."

For getParent() this makes sense to mention.  But it seems unnecessary 
for the other two methods, which are static, and designed to always 
return the indicated classloader.  The getSystemClassLoader() docs go on 
to immediately mention that the system classloader is the default 
delegation parent.


Omitting the phrase makes the spec more concise.

Thanks,
-Brent


Re: RFR 8191173 : (cl) Clarify or remove "for delegation" in ClassLoader spec

2017-11-21 Thread Martin Buchholz
Looks good!

On Tue, Nov 21, 2017 at 12:41 PM, Brent Christian <
brent.christ...@oracle.com> wrote:

> Hi,
>
> Please review my change to this small bit of ClassLoader spec that can be
> tidied up, as noticed by Martin.
>
> Issue:
> https://bugs.openjdk.java.net/browse/JDK-8191173
> Webrev:
> http://cr.openjdk.java.net/~bchristi/8191173/webrev.00/
>
> In java.lang.ClassLoader these methods:
>   getParent()
>   getPlatformClassLoader()
>   getSystemClassLoader()
> all state that the returned classloader is, "for delegation."
>
> For getParent() this makes sense to mention.  But it seems unnecessary for
> the other two methods, which are static, and designed to always return the
> indicated classloader.  The getSystemClassLoader() docs go on to
> immediately mention that the system classloader is the default delegation
> parent.
>
> Omitting the phrase makes the spec more concise.
>
> Thanks,
> -Brent
>


RFR 8191173 : (cl) Clarify or remove "for delegation" in ClassLoader spec

2017-11-21 Thread Brent Christian

Hi,

Please review my change to this small bit of ClassLoader spec that can 
be tidied up, as noticed by Martin.


Issue:
https://bugs.openjdk.java.net/browse/JDK-8191173
Webrev:
http://cr.openjdk.java.net/~bchristi/8191173/webrev.00/

In java.lang.ClassLoader these methods:
  getParent()
  getPlatformClassLoader()
  getSystemClassLoader()
all state that the returned classloader is, "for delegation."

For getParent() this makes sense to mention.  But it seems unnecessary 
for the other two methods, which are static, and designed to always 
return the indicated classloader.  The getSystemClassLoader() docs go on 
to immediately mention that the system classloader is the default 
delegation parent.


Omitting the phrase makes the spec more concise.

Thanks,
-Brent