On 5/06/2015 9:32 AM, Chris Plummer wrote:
Hi David,
Here's an updated webrev:
http://cr.openjdk.java.net/~cjplummer/8054386/webrev.04/
Thanks - should have said updated webrev not necessary :)
Looks good.
David
thanks,
Chris
On 6/3/15 11:29 PM, David Holmes wrote:
Hi Chris,
Hotspot
I guess, there is no need to re-review.
It looks good anyway.
Thanks,
Serguei
On 6/4/15 4:32 PM, Chris Plummer wrote:
Hi David,
Here's an updated webrev:
http://cr.openjdk.java.net/~cjplummer/8054386/webrev.04/
thanks,
Chris
On 6/3/15 11:29 PM, David Holmes wrote:
Hi Chris,
Hotspot
Hi David,
Here's an updated webrev:
http://cr.openjdk.java.net/~cjplummer/8054386/webrev.04/
thanks,
Chris
On 6/3/15 11:29 PM, David Holmes wrote:
Hi Chris,
Hotspot change is good.
Only a couple of style nits with the tests (where are our Java style
guidelines ???). Taking
Hi Chris,
Hotspot change is good.
Only a couple of style nits with the tests (where are our Java style
guidelines ???). Taking CDSJDITest.java as an example:
If you are okay with this line length:
115 public static OutputAnalyzer executeAndLog(ProcessBuilder pb,
String logName) throws
Hi Chris,
I've replied with a thumbs up on another thread.
Thanks,
Serguei
On 6/3/15 4:23 PM, Chris Plummer wrote:
Hi Serguei,
I'll take care of the rename. Can I also put you down for the
ProcessTool.java changes, which are officially being reviewed on
another thread:
Hi Serguei,
I'll take care of the rename. Can I also put you down for the
ProcessTool.java changes, which are officially being reviewed on another
thread:
http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-June/033892.html
thanks,
Chris
On 6/3/15 1:40 AM, serguei.spit...@oracle.com
Chris,
It looks good in general.
I'd suggest to rename the folder:
|| test/com/sun/jdi/CDSJDITests
to:
test/com/sun/jdi/cds
There is no need to spell JDI as it is already a sub-folder of the
com/sun/jdi
and there is no need to spell Tests too as it is in the test repo.
Also, all the folders
[Adding core-libs-dev@openjdk.java.net since this update includes
changes to jdk/test library code]
Please review the updated webrev:
Webrev: http://cr.openjdk.java.net/~cjplummer/8054386/webrev.02/
Bug: https://bugs.openjdk.java.net/browse/JDK-8054386
There were concerns about the new
I'm going to have to separate out the ProcessTool.java changes into a
separate changeset (and CR). In the meantime, feel free to review what I
have below. The code won't be changing at all when I separate out the
ProcessTool.java changes.
thanks,
Chris
On 6/2/15 12:36 AM, Chris Plummer
Changes look good to me.
Misha
On 6/2/2015 11:55 AM, Chris Plummer wrote:
I'm going to have to separate out the ProcessTool.java changes into a
separate changeset (and CR). In the meantime, feel free to review what
I have below. The code won't be changing at all when I separate out
the
10 matches
Mail list logo
