Re: RFR (XS): 8245392: Remove duplication in class redefinition and retransformation specs

2020-05-22 Thread serguei.spit...@oracle.com

Hi Chris,

Thank you for the review!
I'll make it consistent.

Thanks,
Serguei


On 5/22/20 11:33, Chris Plummer wrote:

Hi Serguei,

Just one very minor editing suggestion. Where you have "If 
canUnrestrictedlyRedefineClasses() is false," there is a comma placed 
here, but the previous two bullet items are of a similar form, yet 
have no comma. I suggest you make all 3 consistent. I think either 
with or without a comma is acceptable in this case. From what I read 
if the opening phrase is 4 or fewer words, the comma is optional. I'm 
just suggesting you be consistent.


thanks,

Chris

On 5/21/20 10:02 PM, serguei.spit...@oracle.com wrote:

Please, review a fix for:
https://bugs.openjdk.java.net/browse/JDK-8245392


CSR draft (one CSR reviewer is needed before finalizing it):
https://bugs.openjdk.java.net/browse/JDK-8245433

Webrev:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/redef-spec-dedup.1/src/

Updated specs:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/redef-spec-dedup.1/docs/api/java.instrument/java/lang/instrument/Instrumentation.html
http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/redef-spec-dedup.1/docs/specs/jdwp/jdwp-protocol.html
http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/redef-spec-dedup.1/docs/api/jdk.jdi/com/sun/jdi/VirtualMachine.html

Summary:
  The fix is to replace in Instrumentation, JDI and JDWP spec a 
description of class
  redefinition or retransformation restriction with a link to the 
supporting JVM TI

  function where it has been already documented.
  This spec refactoring should help in cases when new 'unmodifiable 
in redefinition' class file attributes are added.


Testing:
  Built docs and checked the link works as expected.

Thanks,
Serguei






Re: RFR (XS): 8245392: Remove duplication in class redefinition and retransformation specs

2020-05-22 Thread Chris Plummer

  
  
Hi Serguei,
  
  Just one very minor editing suggestion. Where you have "If
  canUnrestrictedlyRedefineClasses() is false," there is a comma
  placed here, but the previous two bullet items are of a similar
  form, yet have no comma. I suggest you make all 3 consistent. I
  think either with or without a comma is acceptable in this case.
  From what I read if the opening phrase is 4 or fewer words, the
  comma is optional. I'm just suggesting you be consistent.
  
  thanks,
  
  Chris
  
  On 5/21/20 10:02 PM, serguei.spit...@oracle.com wrote:


  
  Please, review a fix for:
  https://bugs.openjdk.java.net/browse/JDK-8245392

  
  CSR draft (one CSR reviewer is needed before finalizing
  it):
    https://bugs.openjdk.java.net/browse/JDK-8245433
  
Webrev:
 
http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/redef-spec-dedup.1/src/

Updated specs:
 
http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/redef-spec-dedup.1/docs/api/java.instrument/java/lang/instrument/Instrumentation.html
 
http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/redef-spec-dedup.1/docs/specs/jdwp/jdwp-protocol.html
 
http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/redef-spec-dedup.1/docs/api/jdk.jdi/com/sun/jdi/VirtualMachine.html

Summary:
  The fix is to replace in Instrumentation, JDI and JDWP spec a
description of class
  redefinition or retransformation restriction with a link to
the supporting JVM TI
  function where it has been already documented.
  This spec refactoring should help in cases when new
'unmodifiable in redefinition' class file attributes are added.

Testing:
  Built docs and checked the link works as expected.

Thanks,
Serguei
   

  




Re: RFR (XS): 8245392: Remove duplication in class redefinition and retransformation specs

2020-05-22 Thread serguei.spit...@oracle.com

Thanks a lot, David!
Serguei


On 5/21/20 23:18, David Holmes wrote:

Hi Serguei,

Looks good.

Thanks,
David

On 22/05/2020 4:09 pm, serguei.spit...@oracle.com wrote:

Hi David,

Thank you for the suggestions and corrections!
I've updated the CSR description and regenerated the spec html files 
and webrev.


Thanks,
Serguei


On 5/21/20 22:34, David Holmes wrote:

Hi Serguei,

On 22/05/2020 3:02 pm, serguei.spit...@oracle.com wrote:

Please, review a fix for:
https://bugs.openjdk.java.net/browse/JDK-8245392


CSR draft (one CSR reviewer is needed before finalizing it):
https://bugs.openjdk.java.net/browse/JDK-8245433


I've made some updates and comments on the CSR request. Only minor 
suggestions. Once that is addressed I'll add my review there and here.


Thanks,
David


Webrev:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/redef-spec-dedup.1/src/ 



Updated specs:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/redef-spec-dedup.1/docs/api/java.instrument/java/lang/instrument/Instrumentation.html 

http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/redef-spec-dedup.1/docs/specs/jdwp/jdwp-protocol.html 

http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/redef-spec-dedup.1/docs/api/jdk.jdi/com/sun/jdi/VirtualMachine.html 



Summary:
   The fix is to replace in Instrumentation, JDI and JDWP spec a 
description of class
   redefinition or retransformation restriction with a link to the 
supporting JVM TI

   function where it has been already documented.
   This spec refactoring should help in cases when new 
'unmodifiable in redefinition' class file attributes are added.


Testing:
   Built docs and checked the link works as expected.

Thanks,
Serguei






Re: RFR (XS): 8245392: Remove duplication in class redefinition and retransformation specs

2020-05-22 Thread David Holmes

Hi Serguei,

Looks good.

Thanks,
David

On 22/05/2020 4:09 pm, serguei.spit...@oracle.com wrote:

Hi David,

Thank you for the suggestions and corrections!
I've updated the CSR description and regenerated the spec html files and 
webrev.


Thanks,
Serguei


On 5/21/20 22:34, David Holmes wrote:

Hi Serguei,

On 22/05/2020 3:02 pm, serguei.spit...@oracle.com wrote:

Please, review a fix for:
https://bugs.openjdk.java.net/browse/JDK-8245392


CSR draft (one CSR reviewer is needed before finalizing it):
https://bugs.openjdk.java.net/browse/JDK-8245433


I've made some updates and comments on the CSR request. Only minor 
suggestions. Once that is addressed I'll add my review there and here.


Thanks,
David


Webrev:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/redef-spec-dedup.1/src/ 



Updated specs:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/redef-spec-dedup.1/docs/api/java.instrument/java/lang/instrument/Instrumentation.html 

http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/redef-spec-dedup.1/docs/specs/jdwp/jdwp-protocol.html 

http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/redef-spec-dedup.1/docs/api/jdk.jdi/com/sun/jdi/VirtualMachine.html 



Summary:
   The fix is to replace in Instrumentation, JDI and JDWP spec a 
description of class
   redefinition or retransformation restriction with a link to the 
supporting JVM TI

   function where it has been already documented.
   This spec refactoring should help in cases when new 'unmodifiable 
in redefinition' class file attributes are added.


Testing:
   Built docs and checked the link works as expected.

Thanks,
Serguei




Re: RFR (XS): 8245392: Remove duplication in class redefinition and retransformation specs

2020-05-22 Thread serguei.spit...@oracle.com

Hi David,

Thank you for the suggestions and corrections!
I've updated the CSR description and regenerated the spec html files and 
webrev.


Thanks,
Serguei


On 5/21/20 22:34, David Holmes wrote:

Hi Serguei,

On 22/05/2020 3:02 pm, serguei.spit...@oracle.com wrote:

Please, review a fix for:
https://bugs.openjdk.java.net/browse/JDK-8245392


CSR draft (one CSR reviewer is needed before finalizing it):
https://bugs.openjdk.java.net/browse/JDK-8245433


I've made some updates and comments on the CSR request. Only minor 
suggestions. Once that is addressed I'll add my review there and here.


Thanks,
David


Webrev:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/redef-spec-dedup.1/src/ 



Updated specs:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/redef-spec-dedup.1/docs/api/java.instrument/java/lang/instrument/Instrumentation.html 

http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/redef-spec-dedup.1/docs/specs/jdwp/jdwp-protocol.html 

http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/redef-spec-dedup.1/docs/api/jdk.jdi/com/sun/jdi/VirtualMachine.html 



Summary:
   The fix is to replace in Instrumentation, JDI and JDWP spec a 
description of class
   redefinition or retransformation restriction with a link to the 
supporting JVM TI

   function where it has been already documented.
   This spec refactoring should help in cases when new 'unmodifiable 
in redefinition' class file attributes are added.


Testing:
   Built docs and checked the link works as expected.

Thanks,
Serguei




Re: RFR (XS): 8245392: Remove duplication in class redefinition and retransformation specs

2020-05-21 Thread David Holmes

Hi Serguei,

On 22/05/2020 3:02 pm, serguei.spit...@oracle.com wrote:

Please, review a fix for:
https://bugs.openjdk.java.net/browse/JDK-8245392


CSR draft (one CSR reviewer is needed before finalizing it):
https://bugs.openjdk.java.net/browse/JDK-8245433


I've made some updates and comments on the CSR request. Only minor 
suggestions. Once that is addressed I'll add my review there and here.


Thanks,
David


Webrev:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/redef-spec-dedup.1/src/

Updated specs:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/redef-spec-dedup.1/docs/api/java.instrument/java/lang/instrument/Instrumentation.html
http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/redef-spec-dedup.1/docs/specs/jdwp/jdwp-protocol.html
http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/redef-spec-dedup.1/docs/api/jdk.jdi/com/sun/jdi/VirtualMachine.html

Summary:
   The fix is to replace in Instrumentation, JDI and JDWP spec a 
description of class
   redefinition or retransformation restriction with a link to the 
supporting JVM TI

   function where it has been already documented.
   This spec refactoring should help in cases when new 'unmodifiable in 
redefinition' class file attributes are added.


Testing:
   Built docs and checked the link works as expected.

Thanks,
Serguei