Skip to content

Conversation

@FluoriteCafe-work
Copy link
Contributor

@FluoriteCafe-work FluoriteCafe-work commented Sep 3, 2025

Components for Java engine/dependency upgrade reminder.

@FluoriteCafe-work
Copy link
Contributor Author

@microsoft-github-policy-service agree company="Microsoft"

return Jdtls.getProjects(this.nodeData.uri);
const result = await Jdtls.getProjects(this.nodeData.uri);
PromotionProvider.getInstance().checkJavaVersion(result);
return result;
Copy link
Contributor

@wangmingliang-ms wangmingliang-ms Sep 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. what if this method is called >1 times?
  2. The loadData function should only handle data retrieval. Performing a Java version validation at the same time as data fetching seems quite unreasonable. This is clearly a "side effect" and should be avoided.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what if this method is called >1 times?

Controls are implemented in a downstream function to prevent repeated notifications.

The loadData function should only handle data retrieval.

I'll seek to move the added logic out of loadData().

Copy link
Contributor Author

@FluoriteCafe-work FluoriteCafe-work Sep 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated the PR to check Java version on creation of ProjectNode (new ProjectNode(...)) - Java version is related to this type of node. Outside the scope the Java version data is hard to be read (it's saved in a protected property) without touching the property structure of classes.

@FluoriteCafe-work FluoriteCafe-work changed the title Promote Java Upgrade Tool on projects using old (<21) Java Promote Java Upgrade Tool on projects using old (<21) Java and EOL Spring Sep 3, 2025
@FluoriteCafe-work FluoriteCafe-work force-pushed the feat/java-upgrade-tool-notification branch from 67e3caa to 6adb53b Compare September 4, 2025 06:34
@FluoriteCafe-work FluoriteCafe-work changed the title Promote Java Upgrade Tool on projects using old (<21) Java and EOL Spring Remind users to upgrade old (<21) Java and EOL Spring Boot/Framework versions Sep 8, 2025
@FluoriteCafe-work FluoriteCafe-work force-pushed the feat/java-upgrade-tool-notification branch 2 times, most recently from 6cd7dd9 to 991c776 Compare September 8, 2025 06:30
@FluoriteCafe-work

This comment was marked as outdated.

Copy link
Member

@akaroml akaroml left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please detect whether the App Mod ext is active. If it's inactive, no hint should be shown to the users.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please only make necessary changes to reduce the size of of the PR. In this file, the only change is the inserted line of:

  "contributes.commands.java.view.triggerJavaUpgradeTool": "Upgrade dependencies",

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Formatting are restored in c2c4cf6.

src/constants.ts Outdated
export const DIAGNOSTICS_GROUP_ID_FOR_JAVA_ENGINE = "java-engine";
export const DIAGNOSTICS_NAME_FOR_JAVA_ENGINE = "Java Engine";
export const PROMOTION_DIAGNOSTIC_SOURCE = "Java";
export const EARLIEST_JAVA_VERSION_NOT_TO_PROMPT = 21;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LATEST_JAVA_LTS_VERSION

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 56084e6.

src/constants.ts Outdated

export namespace Upgrade {
export const DIAGNOSTICS_GROUP_ID_FOR_JAVA_ENGINE = "java-engine";
export const DIAGNOSTICS_NAME_FOR_JAVA_ENGINE = "Java Engine";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use "java-upgrade-assistant" and "Java Upgrade Assistant" instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to java:* to align with the java-upgrade extension.

src/constants.ts Outdated
export namespace Upgrade {
export const DIAGNOSTICS_GROUP_ID_FOR_JAVA_ENGINE = "java-engine";
export const DIAGNOSTICS_NAME_FOR_JAVA_ENGINE = "Java Engine";
export const PROMOTION_DIAGNOSTIC_SOURCE = "Java";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Java Upgrade Assistant"

src/extension.ts Outdated
import * as path from "path";
import { commands, Diagnostic, Extension, ExtensionContext, extensions, languages,
Range, tasks, TextDocument, TextEditor, Uri, window, workspace } from "vscode";
import {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please avoid unnecessary changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in c3ee610.

instrumentOperation(ENABLE_AUTO_REFRESH, () => this.enableAutoRefresh())();
} else {
instrumentOperation(DISABLE_AUTO_REFRESH, () => {})();
instrumentOperation(DISABLE_AUTO_REFRESH, () => { })();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid unnecessary changes.

} else {
// in flat view
if (path.extname(uri.fsPath) === ".java" && node.uri &&
Uri.parse(node.uri).fsPath === path.dirname(uri.fsPath)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid unnecessary changes.

);
}

private async checkUpgradableComponents(folder: WorkspaceFolder) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we are already in Project Explorer, we already know all the dependencies and the version of java runtimes. Can we work with the project metadata directly? wihtout parsing the POM/Gradle files manually? CC @jdneo

Copy link
Member

@akaroml akaroml left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, I wasn't expecting so many code changes. The fat implementation of manually parsing everything is the root cause. As commented, please consider work with the project metadata directly. In the metadata, the runtime version and dependency versions should all be available.

@akaroml akaroml requested a review from jdneo September 10, 2025 03:59
Copy link
Member

@akaroml akaroml left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, I wasn't expecting so many code changes. The fat implementation of manually parsing everything is the root cause. As commented, please consider work with the project metadata directly. In the metadata, the runtime version and dependency versions should all be available.

@FluoriteCafe-work FluoriteCafe-work force-pushed the feat/java-upgrade-tool-notification branch from abfd8c7 to fa488ef Compare September 10, 2025 06:23
@FluoriteCafe-work
Copy link
Contributor Author

@akaroml Thanks for the review!

As commented, please consider work with the project metadata directly. In the metadata, the runtime version and dependency versions should all be available.

The code is now working with Jdtls to retrieve all metadata. The extension has already gone through the process once, but it's in a lazy-loading way - only retrieving needed parts (projects and dependencies etc.) when user expands the nodes in tree view. As a result, the PR need to actively call Jdtls by itself to gain a full picture.

Copy link
Contributor

@wangmingliang-ms wangmingliang-ms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@wangmingliang-ms wangmingliang-ms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

jdneo
jdneo previously approved these changes Sep 16, 2025
case UpgradeReason.DEPRECATED:
case UpgradeReason.END_OF_LIFE:
default: {
return `The current project is using ${packageDisplayName} ${currentVersion}, which has reached end of life. Do you want to upgrade to ${suggestedVersionName} (${suggestedVersionDescription})?`;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

“which has reached end of life”
Can we be explicit about the time of "end of life"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed by 4d271c1.

The current project is using Spring Boot 3.1.3, which has reached end of life in 2025-06. [...]

@FluoriteCafe-work FluoriteCafe-work dismissed akaroml’s stale review September 18, 2025 03:17

Consensus has been reached per offline discussion.

@FluoriteCafe-work FluoriteCafe-work merged commit 465d988 into microsoft:main Sep 18, 2025
4 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants