-
Notifications
You must be signed in to change notification settings - Fork 410
Make analysis kinds available for starting status report
#3211
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 changes the init action to process the analysis-kinds input before sending the starting status report, allowing the analysis kinds information to be included in that report.
Key Changes
- The
analysis-kindsinput is now parsed earlier in the init process before the starting status report is sent - A new
initAnalysisKindsfunction centralizes the parsing of analysis kinds and handling of the deprecatedquality-queriesinput - The
initConfigfunction's interface is updated to accept pre-parsed analysis kinds instead of raw input strings
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/init-action.ts | Extracts status report sending to a helper function and moves analysis kinds initialization before the starting status report |
| src/config-utils.ts | Updates interface to accept parsed analysis kinds instead of raw input string, removes inline parsing logic |
| src/config-utils.test.ts | Updates test helper to use parsed analysis kinds array instead of input string |
| src/analyses.ts | Adds new initAnalysisKinds function that consolidates analysis kinds parsing and deprecated input handling |
| src/analyses.test.ts | Adds comprehensive test coverage for the new initAnalysisKinds function |
| lib/init-action.js | Generated JavaScript reflecting the TypeScript changes |
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.
Your alternative suggestion of ignoring the exception and then re-running initAnalysisKinds at the normal time seems more robust, but given that incorrect analysis kinds should be pretty rare, this seems OK too.
…uring the first call
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 reasonable
This PR changes the
initaction so that theanalysis-kindsinput is processed before thestartingstatus report is sent. This allows us to include information about the enabled analysis kinds in that report.One aspect of this that we may want to think about is that, if the
analysis-kindsinput is invalid, we now throw aConfigurationErrorfor that before thestartingreport is sent. This will still get caught and result in auser-errorstatus report, but without a matchingstartingstatus report.Alternatives:
ConfigurationErrors thrown byinitAnalysisKindsand re-throw them after thestartingstatus report has been sent to maintain the normal order of operations.initAnalysisKindsat the normal time.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