On Thu, 24 Sep 2020 23:55:15 GMT, Igor Ignatyev <[email protected]> wrote:
> the patch
> - removes `PropertyResolvingWrapper` from `vmTestbase/nsk/jdb` tests
> - updates `JdbArgumentHandler` to remove `"` from `jdb.option` option
> - reformats code
>
> testing:
> ✅ `vmTestbase/nsk/jdb` on {macosx,linux,windows}-x64
The changes look good, but I think the practice of doing a massive cleanup in
"remove usage of PropertyResolvingWrapper
in vmTestbase/nsk/jdb" PR is misleading. Better to split this in two issues (at
least next time)?
test/hotspot/jtreg/vmTestbase/nsk/jdb/clear/clear004/clear004.java line 123:
> 121: Vector<String> v;
> 122:
> 123: v = new Vector<>();
Coalesce these two lines into `Vector<String> v = new Vector<>()`?
test/hotspot/jtreg/vmTestbase/nsk/jdb/clear/clear004/clear004.java line 142:
> 140: Vector<String> v;
> 141:
> 142: v = new Vector<>();
`Vector<String> v = new Vector<>()`?
test/hotspot/jtreg/vmTestbase/nsk/jdb/locals/locals002/locals002.java line 109:
> 107: {"doubleVar", "2.578", "3.8976"},
> 108: {"objVar", "objVarString", "objArgString"},
> 109: {"arrVar", "int[5]", "int[3]"}
This table looked a bit better before. Maybe commas should move to the left,
but tabbing restored?
test/hotspot/jtreg/vmTestbase/nsk/jdb/regression/b4689395/b4689395.java line
147:
> 145: public b4689395() {
> 146: classFile =
> ClassLoadUtils.getRedefineClassFileName(DEBUGGEE_CLASS);
> 147: if (classFile == null)
Braces, while we are changing these lines anyway?
test/hotspot/jtreg/vmTestbase/nsk/jdb/caught_exception/caught_exception002/caught_exception002.java
line 114:
> 112: Paragrep grep;
> 113: int count;
> 114: Vector<String> v = new Vector<>();
As long as we cleaning up the coding here, the declarations can be moved to the
first use? Here... and everywhere.
-------------
Marked as reviewed by shade (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/350
