On Wed, 4 Sep 2024 11:53:34 GMT, Kevin Walls <[email protected]> wrote:
>> Deprecation annotations and warnings on starting the tool(s).
>> Handle man page in a separate issue.
>
> Kevin Walls has updated the pull request incrementally with one additional
> commit since the last revision:
>
> --connect option text, not a WARNING
src/jdk.hotspot.agent/doc/index.html line 28:
> 26: <p>
> 27: <p><strong>WARNING: <b>jhsdb debugd</b> is deprecated and will be removed
> in a future release.</strong></p>
> 28: <p>
You have an extra `<p>` here. I think the one on line 25 should be removed.
src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/SALauncher.java line 75:
> 73: System.out.println();
> 74: if (canConnectToRemote) {
> 75: System.out.println(" The --connect option is deprecated
> and will be removed in a future release.");
It is ok to add this, but now there is no longer any help message to explain
what `--connect` does.
test/hotspot/jtreg/serviceability/sa/sadebugd/ClhsdbTestConnectArgument.java
line 78:
> 76: System.err.println(out.getStderr());
> 77:
> 78: out.shouldMatch("WARNING: --connect is deprecated");
You might want to consider improving
stderrShouldBeEmptyIgnoreDeprecatedWarnings(). It could, for instance, do a
case insensitive search for "warning" and "deprecated" rather than the more
specific search it is currently doing. Just a suggestion. Don't bother if you
don't think it is worth it.
test/hotspot/jtreg/serviceability/sa/sadebugd/ClhsdbTestConnectArgument.java
line 80:
> 78: out.shouldMatch("WARNING: --connect is deprecated");
> 79: // --connect is deprecated. When removed, can revert to:
> 80: // out.stderrShouldBeEmptyIgnoreDeprecatedWarnings();
I don't think this comment is necessary since this test is going way once
`--connect` is removed. Same with this comment in all the other tests.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20830#discussion_r1744314296
PR Review Comment: https://git.openjdk.org/jdk/pull/20830#discussion_r1744315734
PR Review Comment: https://git.openjdk.org/jdk/pull/20830#discussion_r1744320959
PR Review Comment: https://git.openjdk.org/jdk/pull/20830#discussion_r1744318943
