RFR: 8068: Move JMC 9 to JDK 17 [v5]

Christoph Langer clanger at openjdk.org
Sun May 14 21:25:55 UTC 2023


On Fri, 12 May 2023 10:28:15 GMT, Brice Dutheil <bdutheil at openjdk.org> wrote:

>> This PR builds atop #359 which has a number of improvement.
>> * #359
>> 
>> It removed older platform that didn't support Java17 (see https://wiki.eclipse.org/Eclipse/Installation).
>> 
>> I didn't changed the agent to 17 though.
>
> Brice Dutheil has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Set up maven toolchains

Hi,

thanks for picking this up. I've added a few minor remarks, would be nice if you could check them.

Other than that, I suggest to delete all org.eclipse.jdt.core.prefs files. They are automatically generated by Eclipse according to pom.xml etc. anyway, when importing JMC (maven) projects.

application/tests/org.openjdk.jmc.flightrecorder.controlpanel.ui.test/src/test/java/org/openjdk/jmc/flightrecorder/controlpanel/ui/test/PropertyContentBuilderTest.java line 156:

> 154: 		EventNode threadAllocation = javaStatistics.getEvent(threadAllocationID, PathElementKind.IN_BOTH);
> 155: 		assertOptions(threadAllocation, Arrays.asList("Enabled", "IN_BOTH", "Period", "IN_BOTH")); // "extraTestOption",
> 156: 																									// "IN_CONFIGURATION"

Weird formatting

application/tests/org.openjdk.jmc.flightrecorder.controlpanel.ui.test/src/test/java/org/openjdk/jmc/flightrecorder/controlpanel/ui/test/PropertyContentBuilderTest.java line 160:

> 158: 		EventNode classLoadingStatistics = javaStatistics.getEvent(classLoadingStatisticsID, PathElementKind.IN_BOTH);
> 159: 		assertOptions(classLoadingStatistics, Arrays.asList("Period", "IN_BOTH", "Enabled", "IN_BOTH")); // "enabledButWrongForTest",
> 160: 																											// "IN_CONFIGURATION"

Weird formatting

application/tests/org.openjdk.jmc.flightrecorder.controlpanel.ui.test/src/test/java/org/openjdk/jmc/flightrecorder/controlpanel/ui/test/PropertyContentBuilderTest.java line 197:

> 195: 		EventNode threadAllocation = javaStatistics.getEvent(threadAllocationID, PathElementKind.IN_BOTH);
> 196: 		assertOptions(threadAllocation, Arrays.asList("Enabled", "IN_BOTH", "Period", "IN_BOTH")); // "extraTestOption",
> 197: 																									// "IN_CONFIGURATION"

formatting

application/tests/org.openjdk.jmc.flightrecorder.controlpanel.ui.test/src/test/java/org/openjdk/jmc/flightrecorder/controlpanel/ui/test/PropertyContentBuilderTest.java line 201:

> 199: 		EventNode classLoadingStatistics = javaStatistics.getEvent(classLoadingStatisticsID, PathElementKind.IN_BOTH);
> 200: 		assertOptions(classLoadingStatistics, Arrays.asList("Period", "IN_BOTH", "Enabled", "IN_SERVER")); // "enabledButWrongForTest",
> 201: 																											// "IN_CONFIGURATION"

formatting

application/tests/org.openjdk.jmc.flightrecorder.controlpanel.ui.test/src/test/java/org/openjdk/jmc/flightrecorder/controlpanel/ui/test/PropertyContentBuilderTest.java line 255:

> 253: 		String[] expected = new String[] {"Enabled", "Period", "Stack Trace", "Threshold"};
> 254: 		if (ConnectionToolkit.isJavaVersionAboveOrEqual(getConnectionHandle(), JavaVersionSupport.JDK_16)) {
> 255: 			// probably related to https://bugs.openjdk.java.net/browse/JDK-8257602

URL: bugs.openjdk.org

core/org.openjdk.jmc.common/src/main/java/org/openjdk/jmc/common/version/JavaVersionSupport.java line 71:

> 69: 	public static final JavaVersion JDK_15 = new JavaVersion(15, 0);
> 70: 	public static final JavaVersion JDK_16 = new JavaVersion(16, 0);
> 71: 	public static final JavaVersion JDK_17 = new JavaVersion(17, 0);

Update copyright year in this file (and maybe others, too)

docs/devguide/README.md line 74:

> 72: 
> 73: Next we will import the project which contains the launchers. Select _File | Import…_ and then select _Existing Projects into Workspace_. Find the `configuration/ide/eclipse` folder and click Ok.
> 74: 

Why is the step to import the launchers removed?

-------------

Changes requested by clanger (Committer).

PR Review: https://git.openjdk.org/jmc/pull/482#pullrequestreview-1425604910
PR Review Comment: https://git.openjdk.org/jmc/pull/482#discussion_r1193210284
PR Review Comment: https://git.openjdk.org/jmc/pull/482#discussion_r1193210320
PR Review Comment: https://git.openjdk.org/jmc/pull/482#discussion_r1193210379
PR Review Comment: https://git.openjdk.org/jmc/pull/482#discussion_r1193210409
PR Review Comment: https://git.openjdk.org/jmc/pull/482#discussion_r1193210488
PR Review Comment: https://git.openjdk.org/jmc/pull/482#discussion_r1193210682
PR Review Comment: https://git.openjdk.org/jmc/pull/482#discussion_r1193210969


More information about the jmc-dev mailing list