-
Notifications
You must be signed in to change notification settings - Fork 420
Delete unused exports #3315
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
Delete unused exports #3315
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 removes unused exports and dependencies from the CodeQL Action codebase to reduce maintenance burden and improve code clarity.
Key changes:
- Removed or converted multiple unused exported functions, constants, interfaces, and enums to internal/private visibility
- Removed unused npm dependencies (
@octokit/request-error,octokit) - Updated dependency version constraints for better maintenance
Reviewed Changes
Copilot reviewed 30 out of 31 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/util.ts | Removed unused exported functions (fixInvalidNotificationsInFile, checkSipEnablement, unsafeKeysInvariant) and unused import (@actions/exec) |
| src/upload-lib.ts | Changed getSarifFilePaths from exported to private function |
| src/tracer-config.ts | Changed getTracerConfigForCluster from exported to private async function |
| src/tools-download.ts | Changed STREAMING_HIGH_WATERMARK_BYTES from exported to private constant |
| src/status-report.ts | Changed isFirstPartyAnalysis from exported to private function |
| src/start-proxy.ts | Changed UPDATEJOB_PROXY_URL_PREFIX and getLinkedRelease from exported to private |
| src/setup-codeql.ts | Removed unused exports: CODEQL_DEFAULT_ACTION_REPOSITORY, tryGetBundleVersionFromUrl, tryGetFallbackToolcacheVersion, SetupCodeQLResult interface |
| src/overlay-database-utils.ts | Changed checkOverlayBaseDatabase from exported to private function |
| src/git-utils.ts | Removed unused exported functions (deepenGitHistory, gitFetch, gitRepack) |
| src/environment.ts | Removed unused enum values: DISABLE_DUPLICATE_LOCATION_FIX, IS_SIP_ENABLED, ODASA_TRACER_CONFIGURATION |
| src/dependency-caching.ts | Changed getJavaDependencyDirs from exported to private function |
| src/config-utils.ts | Changed multiple functions from exported to private: getSupportedLanguageMap, getRawLanguagesInRepo, getRawLanguages, getPrimaryAnalysisKind |
| src/codeql.ts | Changed functions from exported to private: getCodeQLForCmd, getGeneratedCodeScanningConfigPath |
| src/cli-errors.ts | Changed cliErrorsConfig from exported to private constant |
| src/api-client.ts | Removed duplicate DisallowedAPIVersionReason enum (kept in util.ts where it's actually used) |
| src/analyses.ts | Changed SARIF_UPLOAD_ENDPOINT enum from exported to private |
| src/actions-util.ts | Changed getRelativeScriptPath from exported to private function |
| package.json | Removed unused dependencies (@octokit/request-error, octokit); changed @types/node version constraint from exact to caret range; reordered eslint in devDependencies list |
| package-lock.json | Updated lockfile to reflect dependency removals and related transitive dependency cleanup |
| lib/*.js | Generated JavaScript files reflecting the TypeScript changes (not reviewed per custom guidelines) |
mbg
left a 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.
Nice clean-up. One comment with an observation, but up to you if you want to act on it or not.
| export interface SetupCodeQLResult { | ||
| codeqlFolder: string; | ||
| toolsDownloadStatusReport?: ToolsDownloadStatusReport; | ||
| toolsSource: ToolsSource; | ||
| toolsVersion: string; | ||
| zstdAvailability: tar.ZstdAvailability; | ||
| } |
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.
It looks like setupCodeQLBundle uses an inline version of this. Might be better to keep and use there, rather than the inline interface?
Risk assessment
For internal use only. Please select the risk level of this change:
Which use cases does this change impact?
Workflow types: n/a
Products: n/a
Environments:
How did/will you validate this change?
.test.tsfiles).pr-checks).I also manually inspected the changes to
lib/*.ts.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?
Are there any special considerations for merging or releasing this change?
Merge / deployment checklist