Respect exclude-from-incremental query tag for diff-informed analysis - #2831
cklin merged 8 commits into
Conversation
Sorry, something went wrong.
This commit adds a defaultQueryFilters field to AugmentationProperties and incorporates its value into the augmented Code Scanning config. However, in this commit defaultQueryFilters is always empty, so there is not yet any actual behavior change.
There was a problem hiding this comment.
Pull Request Overview
This PR ensures that diff-informed analysis respects the "exclude-from-incremental" query tag by adding a default query filter when running on pull requests. It updates the configuration augmentation logic and replaces the legacy diff filtering utilities with the new diff-informed analysis utilities.
- Updated import paths and usage of diff utilities across the codebase.
- Modified the augmentation logic to conditionally add a query filter.
- Adjusted tests to account for the new asynchronous behavior and defaultQueryFilters property.
Reviewed Changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated no comments.
Show a summary per file| File | Description |
|---|---|
| src/upload-lib.ts | Updated import to use diff-informed analysis utils. |
| src/diff-informed-analysis-utils.ts | Added new diff utility functions and removal of diff-filtering-utils. |
| src/diff-filtering-utils.ts | Removed legacy diff filtering functionality. |
| src/config-utils.ts | Updated calculateAugmentation to be asynchronous and add defaultQueryFilters. |
| src/config-utils.test.ts | Modified tests to include the new defaultQueryFilters property. |
| src/codeql.ts | Merged defaultQueryFilters into the Code Scanning config generation. |
| src/analyze.ts | Adjusted diff analysis function calls to use the new utilities. |
| src/analyze-action.ts | Updated diff-informed query run call logic with branches check. |
| lib/upload-lib.js | Updated import path for diff-informed analysis utilities. |
| lib/diff-informed-analysis-utils.js | Added new diff utility functions and exported shouldPerformDiffInformedAnalysis. |
| lib/config-utils.test.js | Test adjustments for asynchronous augmentation calculation. |
| lib/config-utils.js | Updated calculateAugmentation to async and integrated defaultQueryFilters. |
| lib/codeql.js | Injected defaultQueryFilters into the augmented Code Scanning config. |
| lib/analyze.js | Replaced legacy diff utility usage with the new diff-informed approaches. |
| lib/analyze-action.js | Adjusted diff-informed analysis setup to use branch information. |
Tip: If you use Visual Studio Code, you can request a review from Copilot before you push from the "Source Control" tab. Learn more
Sorry, something went wrong.
angelapwen
left a comment
There was a problem hiding this comment.
Logic looks 👍!
Non-blocking nit: could shouldPerformDiffInformedQueries return a boolean and then the branches value be returned elsewhere? Or alternatively maybe we could rename the function something even longer and gnarlier, like getBranchesIfDiffInformedQueriesEnabled 😬
Sorry, something went wrong.
|
Yeah, or just checkDiffInformedQueries — we don't have to explain the return type in the function name, but should is usually associated with a boolean return type. |
Sorry, something went wrong.
This commit renames the original shouldPerformDiffInformedAnalysis(), which returns `PullRequestBranches | undefined`, to getDiffInformedAnalysisBranches(). It also adds a new shouldPerformDiffInformedAnalysis() function that returns boolean. Separating these two functions makes it clear what the intended uses and return values should be for each.
|
Thank you for your suggestions @angelapwen and @henrymercer! I updated the PR, and there are now two helper functions.
|
Sorry, something went wrong.
angelapwen
left a comment
There was a problem hiding this comment.
Thank you 🙇♀️
Sorry, something went wrong.
This PR changes the action to add the following settings to the Code Scanning configuration file when performing diff-informed analysis:
Code Scanning configuration file for non-diff-informed analysis remains unchanged.
Commit-by-commit review recommended. The first few commits are preparatory; they change code structure while preserving existing code behavior. The second-last commit adds the new functionality, and the last commit rebuilds the js files.
Merge / deployment checklist