-
Notifications
You must be signed in to change notification settings - Fork 411
Use uploadSarif rather than uploadFiles in analyze action
#3206
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
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.
Pull Request Overview
This PR refactors the analyze action to use the uploadSarif function instead of making separate calls to uploadFiles for Code Scanning and Code Quality analyses. The change consolidates SARIF upload logic that was previously duplicated between the analyze and upload-sarif actions.
Key changes:
- Replaces two separate
uploadFilescalls with a singleuploadSarifcall that handles both Code Scanning and Code Quality uploads - Changes the upload result tracking from a single
UploadResultto a record mapping analysis kinds to their respective results - Updates all references to the upload results throughout the action to access the appropriate analysis kind
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/analyze-action.ts | Refactors upload logic to use uploadSarif and updates result handling to work with grouped results by analysis kind |
| lib/analyze-action.js | Generated JavaScript code reflecting the TypeScript changes, including new helper functions for SARIF file grouping |
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, with one optional code style comment.
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 looks good. Thanks for adding the feature flag. I think you can update the classification in the PR description to low risk now.
This replaces the two calls to
uploadFilesin theanalyzeaction with one call touploadSarif. We introduceduploadSarifin #3167 for theupload-sarifaction. UsinguploadSarifhere means that we no longer use different implementations of the same logic inanalyzeandupload-sarif.Risk assessment
For internal use only. Please select the risk level of this change:
Which use cases does this change impact?
analysis-kinds: code-scanning).analysis-kinds: code-quality).How did/will you validate this change?
.test.tsfiles).pr-checks).If something goes wrong after this change is released, what are the mitigation and rollback strategies?
How will you know if something goes wrong after this change is released?
Merge / deployment checklist