Re: JDK-8150990: [packager] Research Refactoring Packager

2016-04-28 Thread Kevin Rushforth 

Looks good.

-- Kevin


Chris Bensen wrote:

Ugh, day of mistakes. Here’s the correct link:

http://cr.openjdk.java.net/~cbensen/JDK-8150990/webrev.02/


  

On Apr 28, 2016, at 7:07 AM, Chris Bensen  wrote:

Doesn’t look like it’s been pushed. I revised the change:

http://cr.openjdk.java.net/~cbensen/JDK-8150990/webrev.01/

Chris




On Apr 28, 2016, at 6:55 AM, Chris Bensen  wrote:

If it hasn’t been pushed I’ll change it?

Chris


  

On Apr 27, 2016, at 11:58 PM, Alan Bateman  wrote:



On 27/04/2016 21:44, Kevin Rushforth wrote:


Looks fine to me, too.

Btw, I thought Mandy had earlier suggested jdk.tools.jlink.internal.packager as 
a name, but it's currently jdk.tools.internal.packager (without the jlink). I 
don't care one way or the other and the current one is shorter.
  

Mandy's suggestion sounds right to me as jlink already has internals in 
jdk.tools.jlink.internal.**. If it's temporary then the suggest is probably 
okay although if temporary means to JDK 9 GA then it might be better to change 
it now.

-Alan

Re: JDK-8150990: [packager] Research Refactoring Packager

2016-04-28 Thread Chris Bensen 
Ugh, day of mistakes. Here’s the correct link:

http://cr.openjdk.java.net/~cbensen/JDK-8150990/webrev.02/


> On Apr 28, 2016, at 7:07 AM, Chris Bensen  wrote:
> 
> Doesn’t look like it’s been pushed. I revised the change:
> 
> http://cr.openjdk.java.net/~cbensen/JDK-8150990/webrev.01/
> 
> Chris
> 
> 
>> On Apr 28, 2016, at 6:55 AM, Chris Bensen  wrote:
>> 
>> If it hasn’t been pushed I’ll change it?
>> 
>> Chris
>> 
>> 
>>> On Apr 27, 2016, at 11:58 PM, Alan Bateman  wrote:
>>> 
>>> 
>>> 
>>> On 27/04/2016 21:44, Kevin Rushforth wrote:
 Looks fine to me, too.
 
 Btw, I thought Mandy had earlier suggested 
 jdk.tools.jlink.internal.packager as a name, but it's currently 
 jdk.tools.internal.packager (without the jlink). I don't care one way or 
 the other and the current one is shorter.
>>> Mandy's suggestion sounds right to me as jlink already has internals in 
>>> jdk.tools.jlink.internal.**. If it's temporary then the suggest is probably 
>>> okay although if temporary means to JDK 9 GA then it might be better to 
>>> change it now.
>>> 
>>> -Alan
>> 
>

Re: JDK-8150990: [packager] Research Refactoring Packager

2016-04-28 Thread Chris Bensen 
Doesn’t look like it’s been pushed. I revised the change:

http://cr.openjdk.java.net/~cbensen/JDK-8150990/webrev.01/

Chris


> On Apr 28, 2016, at 6:55 AM, Chris Bensen  wrote:
> 
> If it hasn’t been pushed I’ll change it?
> 
> Chris
> 
> 
>> On Apr 27, 2016, at 11:58 PM, Alan Bateman  wrote:
>> 
>> 
>> 
>> On 27/04/2016 21:44, Kevin Rushforth wrote:
>>> Looks fine to me, too.
>>> 
>>> Btw, I thought Mandy had earlier suggested 
>>> jdk.tools.jlink.internal.packager as a name, but it's currently 
>>> jdk.tools.internal.packager (without the jlink). I don't care one way or 
>>> the other and the current one is shorter.
>> Mandy's suggestion sounds right to me as jlink already has internals in 
>> jdk.tools.jlink.internal.**. If it's temporary then the suggest is probably 
>> okay although if temporary means to JDK 9 GA then it might be better to 
>> change it now.
>> 
>> -Alan
>

Re: JDK-8150990: [packager] Research Refactoring Packager

2016-04-28 Thread Chris Bensen 
If it hasn’t been pushed I’ll change it?

Chris


> On Apr 27, 2016, at 11:58 PM, Alan Bateman  wrote:
> 
> 
> 
> On 27/04/2016 21:44, Kevin Rushforth wrote:
>> Looks fine to me, too.
>> 
>> Btw, I thought Mandy had earlier suggested jdk.tools.jlink.internal.packager 
>> as a name, but it's currently jdk.tools.internal.packager (without the 
>> jlink). I don't care one way or the other and the current one is shorter.
> Mandy's suggestion sounds right to me as jlink already has internals in 
> jdk.tools.jlink.internal.**. If it's temporary then the suggest is probably 
> okay although if temporary means to JDK 9 GA then it might be better to 
> change it now.
> 
> -Alan

Re: JDK-8150990: [packager] Research Refactoring Packager

2016-04-28 Thread Alan Bateman 



On 27/04/2016 21:44, Kevin Rushforth wrote:

Looks fine to me, too.

Btw, I thought Mandy had earlier suggested 
jdk.tools.jlink.internal.packager as a name, but it's currently 
jdk.tools.internal.packager (without the jlink). I don't care one way 
or the other and the current one is shorter.
Mandy's suggestion sounds right to me as jlink already has internals in 
jdk.tools.jlink.internal.**. If it's temporary then the suggest is 
probably okay although if temporary means to JDK 9 GA then it might be 
better to change it now.


-Alan

Re: JDK-8150990: [packager] Research Refactoring Packager

2016-04-27 Thread Kevin Rushforth 

Let's leave it then.

-- Kevin


Chris Bensen wrote:

To be honest I messed up. I noticed when I was building and testing just before 
sending out the review. I kinda liked it after I noticed but I’m good with 
either one.

Chris


  

On Apr 27, 2016, at 1:59 PM, Mandy Chung  wrote:




On Apr 27, 2016, at 1:44 PM, Kevin Rushforth  wrote:

Looks fine to me, too.

Btw, I thought Mandy had earlier suggested jdk.tools.jlink.internal.packager as 
a name, but it's currently jdk.tools.internal.packager (without the jlink). I 
don't care one way or the other and the current one is shorter.
  

I don’t have a preference on either one. This is just temporary and this may go 
away when jlink API is simplified and becomes stable (something to revisit 
later).

Mandy

Re: JDK-8150990: [packager] Research Refactoring Packager

2016-04-27 Thread Chris Bensen 
To be honest I messed up. I noticed when I was building and testing just before 
sending out the review. I kinda liked it after I noticed but I’m good with 
either one.

Chris


> On Apr 27, 2016, at 1:59 PM, Mandy Chung  wrote:
> 
> 
>> On Apr 27, 2016, at 1:44 PM, Kevin Rushforth  
>> wrote:
>> 
>> Looks fine to me, too.
>> 
>> Btw, I thought Mandy had earlier suggested jdk.tools.jlink.internal.packager 
>> as a name, but it's currently jdk.tools.internal.packager (without the 
>> jlink). I don't care one way or the other and the current one is shorter.
> 
> I don’t have a preference on either one. This is just temporary and this may 
> go away when jlink API is simplified and becomes stable (something to revisit 
> later).
> 
> Mandy
>

Re: JDK-8150990: [packager] Research Refactoring Packager

2016-04-27 Thread Kevin Rushforth 

Looks fine to me, too.

Btw, I thought Mandy had earlier suggested 
jdk.tools.jlink.internal.packager as a name, but it's currently 
jdk.tools.internal.packager (without the jlink). I don't care one way or 
the other and the current one is shorter.


-- Kevin


Mandy Chung wrote:

On Apr 27, 2016, at 11:51 AM, Chris Bensen  wrote:

http://cr.openjdk.java.net/~cbensen/JDK-8150990/webrev.00/



This looks okay to me. This is an interim solution to greatly simplify FX coordination with jlink API change during jdk9 development.  


I can sponsor it and push it to jdk9-dev.

Mandy

Re: JDK-8150990: [packager] Research Refactoring Packager

2016-04-27 Thread Mandy Chung 

> On Apr 27, 2016, at 11:51 AM, Chris Bensen  wrote:
> 
> http://cr.openjdk.java.net/~cbensen/JDK-8150990/webrev.00/

This looks okay to me. This is an interim solution to greatly simplify FX 
coordination with jlink API change during jdk9 development.  

I can sponsor it and push it to jdk9-dev.

Mandy

Re: JDK-8150990: [packager] Research Refactoring Packager

2016-04-27 Thread Chris Bensen 

> On Apr 27, 2016, at 11:09 AM, Alan Bateman  wrote:
> 
> 
> 
> On 27/04/2016 19:00, Chris Bensen wrote:
>> Because the Java Packager lives in the JavaFX repository and jlink lives in 
>> the JDK repository, changes to the jlink API have caused a lot of headaches. 
>> This change introduces a new class for jdk.packager in the jdk.jlink module 
>> to coordinate the jlink API changes. This is an interim solution and will go 
>> away at at some point in the future when the jdk.packager can be brought 
>> into the JDK.
>> 
>> Review: https://java.se.oracle.com/code/cru/CR-JDK9DEV-1361 
>> 
>> JIRA: https://bugs.openjdk.java.net/browse/JDK-8150990 
>> 
>> 
>> 
> Chris - this seems to be an Oracle internal link so better to post the patch 
> on cr.openjdk.java.net.
> 
> -Alan.

OK, I’ll do that right after lunch.

Chris

Re: JDK-8150990: [packager] Research Refactoring Packager

2016-04-27 Thread Alan Bateman 



On 27/04/2016 19:00, Chris Bensen wrote:

Because the Java Packager lives in the JavaFX repository and jlink lives in the 
JDK repository, changes to the jlink API have caused a lot of headaches. This 
change introduces a new class for jdk.packager in the jdk.jlink module to 
coordinate the jlink API changes. This is an interim solution and will go away 
at at some point in the future when the jdk.packager can be brought into the 
JDK.

Review: https://java.se.oracle.com/code/cru/CR-JDK9DEV-1361 

JIRA: https://bugs.openjdk.java.net/browse/JDK-8150990 



Chris - this seems to be an Oracle internal link so better to post the 
patch on cr.openjdk.java.net.


-Alan.

JDK-8150990: [packager] Research Refactoring Packager

2016-04-27 Thread Chris Bensen 
Because the Java Packager lives in the JavaFX repository and jlink lives in the 
JDK repository, changes to the jlink API have caused a lot of headaches. This 
change introduces a new class for jdk.packager in the jdk.jlink module to 
coordinate the jlink API changes. This is an interim solution and will go away 
at at some point in the future when the jdk.packager can be brought into the 
JDK.

Review: https://java.se.oracle.com/code/cru/CR-JDK9DEV-1361 

JIRA: https://bugs.openjdk.java.net/browse/JDK-8150990 


Thanks,
Chris