-
Notifications
You must be signed in to change notification settings - Fork 202
Add "View Autofixes" feature for variant analysis results #4065
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
Conversation
This function will increment file numbers when running autofix in a loop
8095989
to
c5f0e5e
Compare
c5f0e5e
to
3ff5d64
Compare
extensions/ql-vscode/src/variant-analysis/variant-analysis-view.ts
Outdated
Show resolved
Hide resolved
Reference codeQL.autofix.path setting instead of env var
@aeisenberg Which comment are you referring to? There were four things I still was planning to address: three comments related to editing parts of |
Oh...there might be more things that I missed. I'm out tomorrow. I think it's also a holiday in the US, so I can take another look on Monday. |
@aeisenberg @koesie10 I've addressed all review comments. Let me know if you want any other changes, especially regarding the environment variables. |
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.
Thanks, this looks good to me. I have some optional comments, but I don't think any of those are necessary.
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.
Looks generally good. I see one area where I think you can simplify things.
@koesie10 @aeisenberg I've responded to all new review comments. Thanks for the thorough reviews! 🙇 |
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.
LGTM, thanks!
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.
Great job!
I haven't tried this locally, but the code looks good. It's likely that the failing E2E job is transient. I'll kick it off again and hopefully it will work.
@aeisenberg There's a known issue with the E2E tests failing since end of March. The issue tracking it is in the private code-scanning repo. So I think this PR can be merged despite that test failure? |
I'm fine with that. |
Description
Adds a "View Autofixes" feature for variant analysis results. This is a canary-only feature.
Generates autofixes for selected variant analysis results and opens the resulting autofix outputs in a markdown file.
Activated via the "View Autofixes" button in the variant analysis webview or via right-clicking the variant analysis query history item.
At a high-level, the implementation:
Commit-by-commit review recommended. Let me know if you want me to split anything onto a separate PR.
Consideration
queryHelpOverrideDirectory
to the newgo/pkg/autofix/prompt/qhelps
directory in that PR.Testing