Re: Review request: 8055230: Rename attach provider implementation class

2014-08-26 Thread Mandy Chung 


On 8/26/2014 3:59 AM, Staffan Larsen wrote:

There are some differences in the AttachProvideImpl files. Most notably the 
windows version is very different. Linux, Bsd and Aix are the same. The Solaris 
version has a minor difference in the  “public String type()” implementation. 
The Linux, Bsd, Aix and Solaris versions could probably be unified.


I would leave this as it is and further unification can be done
as future cleanup.  Thanks for all the review.

Mandy



/Staffan

On 26 aug 2014, at 12:40, Daniel Fuchs  wrote:


Hi Mandy,

I'm seeing some small differences in the various VirtualMachineImpl.java
files, but if I'm not mistaken, all the new AttachProviderImpl.java look
all the same. I wonder if the patch could be further simplified by
moving AttachProviderImpl.java to jdk.attach/share/classes (unless
there's a reason to keep all the different copies)...

best regards,

-- daniel

On 8/26/14 6:29 AM, Mandy Chung wrote:

Webrev:
http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8055230/

This patch renames the class name of attach provider implementation class
to be the same for all platforms.  This simplifies the build logic and
removes the need for generating the service config file at build time.

The files renamed are
unix/classes/sun/tools/attach/${OS}VirtualMachine.java
  -> ${OS}/classes/sun/tools/attach/VirtualMachineImpl.java
unix/classes/sun/tools/attach/${OS}AttachProvider.java
  -> ${OS}/classes/sun/tools/attach/VirtualMachineImpl.java

${OS}/classes/sun/tools/attach/${OS}VirtualMachine.java
  -> ${OS}/classes/sun/tools/attach/VirtualMachineImpl.java
${OS}/classes/sun/tools/attach/${OS}AttachProvider.java
  -> ${OS}/classes/sun/tools/attach/VirtualMachineImpl.java

and also corresponding native files.

Mandy

Re: Review request: 8055230: Rename attach provider implementation class

2014-08-26 Thread Daniel Fuchs 

On 8/26/14 12:59 PM, Staffan Larsen wrote:

There are some differences in the AttachProvideImpl files. Most notably the 
windows version is very different. Linux, Bsd and Aix are the same. The Solaris 
version has a minor difference in the  “public String type()” implementation. 
The Linux, Bsd, Aix and Solaris versions could probably be unified.


Ah - right - I missed those. Forget my comment then!

Thanks Staffan!

-- daniel



/Staffan

On 26 aug 2014, at 12:40, Daniel Fuchs  wrote:


Hi Mandy,

I'm seeing some small differences in the various VirtualMachineImpl.java
files, but if I'm not mistaken, all the new AttachProviderImpl.java look
all the same. I wonder if the patch could be further simplified by
moving AttachProviderImpl.java to jdk.attach/share/classes (unless
there's a reason to keep all the different copies)...

best regards,

-- daniel

On 8/26/14 6:29 AM, Mandy Chung wrote:

Webrev:
http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8055230/

This patch renames the class name of attach provider implementation class
to be the same for all platforms.  This simplifies the build logic and
removes the need for generating the service config file at build time.

The files renamed are
unix/classes/sun/tools/attach/${OS}VirtualMachine.java
  -> ${OS}/classes/sun/tools/attach/VirtualMachineImpl.java
unix/classes/sun/tools/attach/${OS}AttachProvider.java
  -> ${OS}/classes/sun/tools/attach/VirtualMachineImpl.java

${OS}/classes/sun/tools/attach/${OS}VirtualMachine.java
  -> ${OS}/classes/sun/tools/attach/VirtualMachineImpl.java
${OS}/classes/sun/tools/attach/${OS}AttachProvider.java
  -> ${OS}/classes/sun/tools/attach/VirtualMachineImpl.java

and also corresponding native files.

Mandy

Re: Review request: 8055230: Rename attach provider implementation class

2014-08-26 Thread Staffan Larsen 
There are some differences in the AttachProvideImpl files. Most notably the 
windows version is very different. Linux, Bsd and Aix are the same. The Solaris 
version has a minor difference in the  “public String type()” implementation. 
The Linux, Bsd, Aix and Solaris versions could probably be unified.

/Staffan

On 26 aug 2014, at 12:40, Daniel Fuchs  wrote:

> Hi Mandy,
> 
> I'm seeing some small differences in the various VirtualMachineImpl.java
> files, but if I'm not mistaken, all the new AttachProviderImpl.java look
> all the same. I wonder if the patch could be further simplified by
> moving AttachProviderImpl.java to jdk.attach/share/classes (unless
> there's a reason to keep all the different copies)...
> 
> best regards,
> 
> -- daniel
> 
> On 8/26/14 6:29 AM, Mandy Chung wrote:
>> Webrev:
>>http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8055230/
>> 
>> This patch renames the class name of attach provider implementation class
>> to be the same for all platforms.  This simplifies the build logic and
>> removes the need for generating the service config file at build time.
>> 
>> The files renamed are
>>unix/classes/sun/tools/attach/${OS}VirtualMachine.java
>>  -> ${OS}/classes/sun/tools/attach/VirtualMachineImpl.java
>>unix/classes/sun/tools/attach/${OS}AttachProvider.java
>>  -> ${OS}/classes/sun/tools/attach/VirtualMachineImpl.java
>> 
>>${OS}/classes/sun/tools/attach/${OS}VirtualMachine.java
>>  -> ${OS}/classes/sun/tools/attach/VirtualMachineImpl.java
>>${OS}/classes/sun/tools/attach/${OS}AttachProvider.java
>>  -> ${OS}/classes/sun/tools/attach/VirtualMachineImpl.java
>> 
>> and also corresponding native files.
>> 
>> Mandy
>> 
>> 
>

Re: Review request: 8055230: Rename attach provider implementation class

2014-08-26 Thread Daniel Fuchs 

Hi Mandy,

I'm seeing some small differences in the various VirtualMachineImpl.java
files, but if I'm not mistaken, all the new AttachProviderImpl.java look
all the same. I wonder if the patch could be further simplified by
moving AttachProviderImpl.java to jdk.attach/share/classes (unless
there's a reason to keep all the different copies)...

best regards,

-- daniel

On 8/26/14 6:29 AM, Mandy Chung wrote:

Webrev:
http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8055230/

This patch renames the class name of attach provider implementation class
to be the same for all platforms.  This simplifies the build logic and
removes the need for generating the service config file at build time.

The files renamed are
unix/classes/sun/tools/attach/${OS}VirtualMachine.java
  -> ${OS}/classes/sun/tools/attach/VirtualMachineImpl.java
unix/classes/sun/tools/attach/${OS}AttachProvider.java
  -> ${OS}/classes/sun/tools/attach/VirtualMachineImpl.java

${OS}/classes/sun/tools/attach/${OS}VirtualMachine.java
  -> ${OS}/classes/sun/tools/attach/VirtualMachineImpl.java
${OS}/classes/sun/tools/attach/${OS}AttachProvider.java
  -> ${OS}/classes/sun/tools/attach/VirtualMachineImpl.java

and also corresponding native files.

Mandy

Re: Review request: 8055230: Rename attach provider implementation class

2014-08-26 Thread Erik Joelsson 

Nice!

/Erik

On 2014-08-26 06:29, Mandy Chung wrote:

Webrev:
   http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8055230/

This patch renames the class name of attach provider implementation class
to be the same for all platforms.  This simplifies the build logic and
removes the need for generating the service config file at build time.

The files renamed are
   unix/classes/sun/tools/attach/${OS}VirtualMachine.java
 -> ${OS}/classes/sun/tools/attach/VirtualMachineImpl.java
   unix/classes/sun/tools/attach/${OS}AttachProvider.java
 -> ${OS}/classes/sun/tools/attach/VirtualMachineImpl.java

   ${OS}/classes/sun/tools/attach/${OS}VirtualMachine.java
 -> ${OS}/classes/sun/tools/attach/VirtualMachineImpl.java
   ${OS}/classes/sun/tools/attach/${OS}AttachProvider.java
 -> ${OS}/classes/sun/tools/attach/VirtualMachineImpl.java

and also corresponding native files.

Mandy

Re: Review request: 8055230: Rename attach provider implementation class

2014-08-26 Thread Chris Hegarty 

On 26 Aug 2014, at 08:26, Alan Bateman  wrote:

> On 26/08/2014 05:29, Mandy Chung wrote:
>> Webrev:
>>   http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8055230/
>> 
>> This patch renames the class name of attach provider implementation class
>> to be the same for all platforms.  This simplifies the build logic and
>> removes the need for generating the service config file at build time.
> This looks good, nice to see the benefits of the new layout.

+1.  Wow this is nice, and the knock on simplification in the build logic is a 
bonus.

-Chris.

> -Alan.

Re: Review request: 8055230: Rename attach provider implementation class

2014-08-26 Thread Alan Bateman 

On 26/08/2014 05:29, Mandy Chung wrote:

Webrev:
   http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8055230/

This patch renames the class name of attach provider implementation class
to be the same for all platforms.  This simplifies the build logic and
removes the need for generating the service config file at build time.

This looks good, nice to see the benefits of the new layout.

-Alan.

Re: Review request: 8055230: Rename attach provider implementation class

2014-08-25 Thread Staffan Larsen 
Ahh. The simplicity!

Looks good!

Thanks,
/Staffan

On 26 aug 2014, at 06:29, Mandy Chung  wrote:

> Webrev:
>   http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8055230/
> 
> This patch renames the class name of attach provider implementation class
> to be the same for all platforms.  This simplifies the build logic and
> removes the need for generating the service config file at build time.
> 
> The files renamed are
>   unix/classes/sun/tools/attach/${OS}VirtualMachine.java
> -> ${OS}/classes/sun/tools/attach/VirtualMachineImpl.java
>   unix/classes/sun/tools/attach/${OS}AttachProvider.java
> -> ${OS}/classes/sun/tools/attach/VirtualMachineImpl.java
> 
>   ${OS}/classes/sun/tools/attach/${OS}VirtualMachine.java
> -> ${OS}/classes/sun/tools/attach/VirtualMachineImpl.java
>   ${OS}/classes/sun/tools/attach/${OS}AttachProvider.java
> -> ${OS}/classes/sun/tools/attach/VirtualMachineImpl.java
> 
> and also corresponding native files.
> 
> Mandy
> 
>

Re: Review request: 8055230: Rename attach provider implementation class

2014-08-25 Thread David Holmes 

I like this change! :)

Looks good to me.

Thanks Mandy!

David
-

On 26/08/2014 2:29 PM, Mandy Chung wrote:

Webrev:
http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8055230/

This patch renames the class name of attach provider implementation class
to be the same for all platforms.  This simplifies the build logic and
removes the need for generating the service config file at build time.

The files renamed are
unix/classes/sun/tools/attach/${OS}VirtualMachine.java
  -> ${OS}/classes/sun/tools/attach/VirtualMachineImpl.java
unix/classes/sun/tools/attach/${OS}AttachProvider.java
  -> ${OS}/classes/sun/tools/attach/VirtualMachineImpl.java

${OS}/classes/sun/tools/attach/${OS}VirtualMachine.java
  -> ${OS}/classes/sun/tools/attach/VirtualMachineImpl.java
${OS}/classes/sun/tools/attach/${OS}AttachProvider.java
  -> ${OS}/classes/sun/tools/attach/VirtualMachineImpl.java

and also corresponding native files.

Mandy

Review request: 8055230: Rename attach provider implementation class

2014-08-25 Thread Mandy Chung 

Webrev:
   http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8055230/

This patch renames the class name of attach provider implementation class
to be the same for all platforms.  This simplifies the build logic and
removes the need for generating the service config file at build time.

The files renamed are
   unix/classes/sun/tools/attach/${OS}VirtualMachine.java
 -> ${OS}/classes/sun/tools/attach/VirtualMachineImpl.java
   unix/classes/sun/tools/attach/${OS}AttachProvider.java
 -> ${OS}/classes/sun/tools/attach/VirtualMachineImpl.java

   ${OS}/classes/sun/tools/attach/${OS}VirtualMachine.java
 -> ${OS}/classes/sun/tools/attach/VirtualMachineImpl.java
   ${OS}/classes/sun/tools/attach/${OS}AttachProvider.java
 -> ${OS}/classes/sun/tools/attach/VirtualMachineImpl.java

and also corresponding native files.

Mandy