Skip to content

Conversation

@inesgomas
Copy link
Contributor

No description provided.

@inesgomas inesgomas requested a review from alerizzo April 8, 2025 09:35
@codacy-production
Copy link

codacy-production bot commented Apr 8, 2025

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
-1.22% 0.00%
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (9e2de19) 3047 509 16.70%
Head commit (cb9df0c) 3286 (+239) 509 (+0) 15.49% (-1.22%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#43) 284 0 0.00%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

@inesgomas inesgomas force-pushed the setup-cli-v2-CY-7445 branch from b0a6a9f to 36417ec Compare April 8, 2025 10:52
@inesgomas inesgomas force-pushed the setup-cli-v2-CY-7445 branch from 1e812e4 to 0eef37a Compare April 8, 2025 15:57
* added temp file analysis and sarif processing, still in progress

* refactor diagnostics to support both API and CLI

* add analysis of unsaved files

* local analysis working with latest version of cli v2
@alerizzo alerizzo changed the title Setup CLI v2 CY-7445 feat: Integrate with CLI v2 CY-7445 Apr 10, 2025
@alerizzo alerizzo marked this pull request as ready for review April 10, 2025 16:45
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.

Files not reviewed (1)
  • package.json: Language not supported

}

const documentLines = document.getText().split('\n')
private updateApiISsuesPositions(document: vscode.TextDocument) {
Copy link

Copilot AI Apr 10, 2025

Choose a reason for hiding this comment

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

There is a typo in the method name 'updateApiISsuesPositions'. It should be spelled 'updateApiIssuesPositions' to improve clarity.

Suggested change
private updateApiISsuesPositions(document: vscode.TextDocument) {
private updateApiIssuesPositions(document: vscode.TextDocument) {

Copilot uses AI. Check for mistakes.
GitProvider.instance?.onDidChangeTextDocument((e) => {
this.updatePositions(e.document)
GitProvider.instance?.onDidChangeTextDocument(async (e) => {
// avoid if the ddocument is a .git file
Copy link

Copilot AI Apr 10, 2025

Choose a reason for hiding this comment

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

The comment contains a typo ('ddocument'); it should be corrected to 'document'.

Suggested change
// avoid if the ddocument is a .git file
// avoid if the document is a .git file

Copilot uses AI. Check for mistakes.
const documentApiIssues = this._currentApiIssues[document.uri.fsPath] || []
const documentCliIssues = this._currentCliIssues[document.uri.fsPath] || []

// remove API issues found in the CLI issues
Copy link
Contributor Author

Choose a reason for hiding this comment

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

so basically we are prioritizing CLI issues, correct?

Copy link
Contributor

Choose a reason for hiding this comment

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

yup, those should be more up to date

@inesgomas
Copy link
Contributor Author

Can't approve but looks good Ale :D

@alerizzo alerizzo merged commit 1815946 into main Apr 11, 2025
5 checks passed
@alerizzo alerizzo deleted the setup-cli-v2-CY-7445 branch April 11, 2025 10:02
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.

3 participants