RFR (S): (7u60,8u20,9) 8032771 Need test to cover JDK-8013132 (original) (raw)
Bengt Rutisson bengt.rutisson at oracle.com
Tue Jan 28 11:27:00 UTC 2014
- Previous message (by thread): RFR (S): (7u60,8u20,9) 8032771 Need test to cover JDK-8013132
- Next message (by thread): RFR (S): (7u60,8u20,9) 8032771 Need test to cover JDK-8013132
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
Hi Thomas,
On 2014-01-28 11:38, Thomas Schatzl wrote:
Hi Bengt,
thanks for the review. On Tue, 2014-01-28 at 10:43 +0100, Bengt Rutisson wrote: Hi Thomas,
Looks good. Two minor nits: The copyright year is 2013. Fixed.
I really don't like discussing copyright year. But since this is a new file I think the copyright year should be just 2014. Not "2013, 2014".
The method printJavaOpts() seems to provide useful information. Maybe it should be part of ProcessTools.createJavaProcessBuilder()? I removed the method and all related references. This functionality is already provided by ProcessTools.createJavaProcessBuilder(). It got introduced by me basing this patch from TestVerifyDuringStartup.java, and then refactoring. Not sure why the original code contains this additional printout. New webrev: http://cr.openjdk.java.net/~tschatzl/8032771/webrev.1
Looks good. Thanks for cleaning this up.
Bengt
Thomas
- Previous message (by thread): RFR (S): (7u60,8u20,9) 8032771 Need test to cover JDK-8013132
- Next message (by thread): RFR (S): (7u60,8u20,9) 8032771 Need test to cover JDK-8013132
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]