Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue455 refresh issue due to pom generated in target on building the project #462

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

JoshwinThomasIBM
Copy link
Contributor

Fixes #455

@@ -32,7 +32,7 @@ export const COMMAND_AND_PROJECT_TYPE_MAP: { [command: string]: string[] } = {
"gradle":[ LIBERTY_GRADLE_PROJECT, LIBERTY_GRADLE_PROJECT_CONTAINER],
"liberty.dev.debug": [LIBERTY_MAVEN_PROJECT, LIBERTY_GRADLE_PROJECT, LIBERTY_MAVEN_PROJECT_CONTAINER, LIBERTY_GRADLE_PROJECT_CONTAINER],
};
export const EXCLUDED_DIR_PATTERN = "**/{bin,classes,target}/**";
export const EXCLUDED_DIR_PATTERN = "**/{bin,classes,target,build}/**";
Copy link
Contributor

Choose a reason for hiding this comment

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

What is EXCLUDED_DIR_PATTERN used for? I see that target was already included, so this must not be working for the refresh scenario. What is the reason for adding build here?

// Listens for any new folders are added to the workspace
context.subscriptions.push(vscode.workspace.onDidChangeWorkspaceFolders((event) => {
// Listens for any new folders are added to the workspace
context.subscriptions.push(vscode.workspace.onDidChangeWorkspaceFolders((event) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please do not modify spacing or formatting on existing code, unless it is in a separate PR specific to formatting changes. It is not immediately clear if there are any functional changes in the middle of formatting changes, which makes code more difficult to review and makes it more difficult for others to understand when looking back at code history.

* @returns
*/
export function isInTargetOrBuild(filePath: string): boolean {
return filePath.includes('/target/') || filePath.includes('/build/');
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is valid if it is just any "target" or "build" dir. We need to ensure the file/path is in a subdirectory of the project's "target" or "build" directory (i.e. the "target" or "build" directory located in the root project folder).

projectProvider.refresh();
});
}

Copy link
Contributor

Choose a reason for hiding this comment

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

I am not entirely sure about this approach, since we are still watching the "target" and "build" dirs. It would be better if we could find a way to ignore them completely.

Copy link
Contributor

Choose a reason for hiding this comment

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

can we add '&&' operator in vscode.workspace.createFileSystemWatcher?
Did conditions like this work '**/pom.xml && !inside target folder' ?

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.

"Refreshing Liberty Dashboard projects" kicks in all the time
3 participants