-
Notifications
You must be signed in to change notification settings - Fork 5
Release/26.1.4 #278
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
base: main
Are you sure you want to change the base?
Release/26.1.4 #278
Conversation
…es-with-constants aadarsh-st/SK-2495 replace hardcode values with constants
|
✅ Gitleaks Findings: No secrets detected. Safe to proceed! |
|
Semgrep Findings: Issues with Error level severity are found (Error is Highest severity in Semgrep), Please resolve the issues before merging. |
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 codebase to replace magic strings with named constants, improving maintainability and consistency. The changes also introduce ESLint configuration with TypeScript support to enforce camelCase naming and prevent the use of string literals for object access.
Changes:
- Introduced new constant groups (SDK, SKYFLOW, CONFIG, HTTP_HEADER, CONTENT_TYPE, ENCODING_TYPE, etc.) to replace hardcoded string literals throughout the codebase
- Updated ESLint configuration to use typescript-eslint with rules enforcing camelCase and restricting magic strings
- Added eslint-disable comments for unavoidable camelCase violations (e.g., external library usage with snake_case)
Reviewed changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/utils/index.ts |
Introduced constant groups to centralize magic strings |
src/vault/skyflow/index.ts |
Replaced string literals with CONFIG constants |
src/vault/controller/vault/index.ts |
Replaced string literals with SDK, SKYFLOW, CONTENT_TYPE, and ENCODING_TYPE constants |
src/vault/controller/detect/index.ts |
Replaced string literals with SDK, FILE_PROCESSING, DETECT_STATUS, and ENCODING_TYPE constants |
src/vault/controller/connections/index.ts |
Replaced string literals with SDK, SKYFLOW, REQUEST, and CONTENT_TYPE constants |
src/vault/client/index.ts |
Replaced string literals with HTTP_HEADER, CONTENT_TYPE, BOOLEAN_STRING, and HTTP_STATUS_CODE constants |
src/utils/validations/index.ts |
Replaced string literals with CONFIG, SKYFLOW, and API_KEY constants |
src/utils/jwt-utils/index.ts |
Added eslint-disable comment for jwt_decode import |
src/service-account/index.ts |
Replaced string literals with HTTP_HEADER, CONTENT_TYPE, HTTP_STATUS_CODE, JWT, and ENCODING_TYPE constants |
test/vault/utils/utils.test.js |
Added eslint-disable comment for jwt_decode import |
test/vault/utils/jwt-utils/jwt.test.js |
Added eslint-disable comment for jwt_decode import |
test/vault/controller/vault.test.js |
Updated mock to include new constant structures |
test/vault/controller/detect.test.js |
Updated mock to include new constants and improved test structure with better documentation |
test/vault/controller/connection.test.js |
Updated references to use SDK and SKYFLOW constant groups |
src/vault/model/options/deidentify-file/bleep-audio/index.ts |
Added eslint-disable comment for camelCase |
samples/vault-api/data-residency.ts |
Added eslint-disable comment for skyflow_id destructuring |
package.json |
Updated version, added ESLint dependencies, and modified lint scripts to include TypeScript files |
eslint.config.mjs |
Created new ESLint configuration with TypeScript support and custom rules |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
✅ Gitleaks Findings: No secrets detected. Safe to proceed! |
|
✅ Gitleaks Findings: No secrets detected. Safe to proceed! |
|
Semgrep Findings: Issues with Error level severity are found (Error is Highest severity in Semgrep), Please resolve the issues before merging. |
1 similar comment
|
Semgrep Findings: Issues with Error level severity are found (Error is Highest severity in Semgrep), Please resolve the issues before merging. |
No description provided.