-
Notifications
You must be signed in to change notification settings - Fork 1
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
NEW (Extension) @W-15639759@ Introduce a new command to run graph engine at the project level #100
Conversation
…ine at the project level
@@ -162,6 +166,10 @@ | |||
"command": "sfca.runDfaOnSelectedMethod", | |||
"when": "false" | |||
}, | |||
{ | |||
"command": "sfca.runDfa", | |||
"when": "false" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm setting this as false so the new command will not show up until Delta run is fully complete. To test this locally, you'll have to change this to true.
src/extension.ts
Outdated
return; | ||
}); | ||
|
||
const projectDir: string[] = vscode.workspace.workspaceFolders?.map(folder => folder.uri.path); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
workspaceRoot is now deprecated and this seems to be the way to get the project directory's root. I couldn't get much info on this from the docs, but I have tested this and it works as expected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we move this logic inside of targeting.getProjectDir() and make currentFile an optional input argument?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's good idea. Done.
src/extension.ts
Outdated
TelemetryService.sendExtensionActivationEvent(extensionHrStart); | ||
outputChannel.appendLine(`Extension sfdx-code-analyzer-vscode activated.`); | ||
return Promise.resolve(context); | ||
} | ||
|
||
async function _runDfa(context: vscode.ExtensionContext, outputChannel: vscode.LogOutputChannel) { | ||
const choice = await vscode.window.showQuickPick( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this prompt show up every time - even when you haven't ran before? Ideally it should only show up when we have ran already with violations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed in the standup, took care of this to show only when cache exists.
src/extension.ts
Outdated
|
||
const projectDir: string[] = vscode.workspace.workspaceFolders?.map(folder => folder.uri.path); | ||
if (projectDir.length === 0) { | ||
void vscode.window.showWarningMessage('***No project directory could be identified. Not proceeding with DFA run.***'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When is this possible? Is it when someone opens up vscode for a file and then closes the file or something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This happens when you haven't opened up any project and still try to run SFCA commands. This is something we should fix at a later point. For example, some of the other extensions make sure a sfdx project is loaded before showing their commands.
src/extension.ts
Outdated
return; | ||
}); | ||
|
||
const projectDir: string[] = vscode.workspace.workspaceFolders?.map(folder => folder.uri.path); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we move this logic inside of targeting.getProjectDir() and make currentFile an optional input argument?
No description provided.