search

keyshade-xyz / keyshade

Realtime secret and configuration management tool, with the best in class security and seamless integration support
https://keyshade.xyz
Mozilla Public License 2.0
196 stars 96 forks source link

feat(cli): Improved the DX for list profile #334

Closed kriptonian1 closed 2 months ago

kriptonian1 commented 2 months ago

User description

Description

Give a summary of the change that you have made

Fixes #[ISSUENO]

Dependencies

Mention any dependencies/packages used

Future Improvements

Mention any improvements to be done in future related to any file/feature

Mentions

Mention and tag the people

Screenshots of relevant screens

Add screenshots of relevant screens

Developer's checklist

If changes are made in the code:

Documentation Update

PR Type

Enhancement, Dependencies

Description

Changes walkthrough 📝

Relevant files
Enhancement
list.profile.ts
Enhance profile listing with table format and colors         
apps/cli/src/commands/profile/list.profile.ts
  • Added cli-table and colors for enhanced table display.
  • Replaced Logger with formatted table output for profiles.
  • Improved verbose output to include API Key and Base URL.
    		•  +50/-13 
    create.profile.ts
    Update prompt message for API key input                                   
    apps/cli/src/commands/profile/create.profile.ts - Changed prompt message from "private key" to "API key".
    		+1/-1     
    Dependencies
    package.json
    Add dependencies and dev script for CLI enhancements         
    apps/cli/package.json
  • Added cli-table, colors, and chalk dependencies.
  • Added @types/cli-table to devDependencies.
  • Added dev script for building and running the project.
    		•  +10/-3   
    Additional files (token-limit)
    pnpm-lock.yaml
    ...                                                                                                           
    pnpm-lock.yaml ...
    		+9305/-11349

    💡 PR-Agent usage: Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    codiumai-pr-agent-free[bot] commented 2 months ago

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Key issues to review

    **Possible Bug:** The use of `require` instead of ES6 import statements in TypeScript files (`list.profile.ts`) might not align with the project's coding guidelines, especially if the project is using ES6 modules elsewhere. **Code Quality:** Inline disabling of ESLint rules (`// eslint-disable-next-line`) for the `require` statements suggests potential issues with the project's linting configuration or module resolution strategy. **Performance Concern:** The dynamic creation of tables and handling of profiles in the `list.profile.ts` might lead to performance issues if the number of profiles is large. Consider optimizing the data handling or using pagination.
    codiumai-pr-agent-free[bot] commented 2 months ago

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Add a check to ensure profiles is not empty before proceeding to create and print the table ___ **Add a check to ensure profiles is not empty before proceeding to create and print the
    table to avoid potential runtime errors.** [apps/cli/src/commands/profile/list.profile.ts [35-37]](https://github.com/keyshade-xyz/keyshade/pull/334/files#diff-4d80b74d95d3f96b1b5343ce7ba0f0aec77ca5f627917b38d1626f9eefb00e9aR35-R37) ```diff const profiles = await fetchProfileConfig() +if (!profiles || Object.keys(profiles).length === 0) { + console.log('No profiles found.') + return +} const defaultProfile = profiles.default delete profiles.default ``` - [ ] **Apply this suggestion**
    Suggestion importance[1-10]: 9 Why: This suggestion addresses a potential runtime error which is crucial for robustness and reliability of the application, making it a highly valuable improvement.
    		9
    Best practice
    Replace require statements with import statements for better consistency and type checking ___ **Instead of using require statements, use import statements for cli-table and colors to
    maintain consistency and leverage TypeScript's type checking.** [apps/cli/src/commands/profile/list.profile.ts [7-10]](https://github.com/keyshade-xyz/keyshade/pull/334/files#diff-4d80b74d95d3f96b1b5343ce7ba0f0aec77ca5f627917b38d1626f9eefb00e9aR7-R10) ```diff -// eslint-disable-next-line @typescript-eslint/no-var-requires -const Table = require('cli-table') -// eslint-disable-next-line @typescript-eslint/no-var-requires -const colors = require('colors/safe') +import Table from 'cli-table' +import colors from 'colors/safe' ``` - [ ] **Apply this suggestion**
    Suggestion importance[1-10]: 8 Why: The suggestion correctly identifies a best practice in TypeScript to use `import` instead of `require` for better type safety and consistency, which is crucial in a TypeScript project.
    		8
    Maintainability
    Combine the creation of profileList arrays to reduce code duplication and improve readability ___ **Combine the creation of profileList arrays to reduce code duplication and improve
    readability.** [apps/cli/src/commands/profile/list.profile.ts [59-65]](https://github.com/keyshade-xyz/keyshade/pull/334/files#diff-4d80b74d95d3f96b1b5343ce7ba0f0aec77ca5f627917b38d1626f9eefb00e9aR59-R65) ```diff -const profileList = [] -Object.keys(profiles).forEach((profile) => { - profileList.push([ - `${defaultProfile === profile ? `${profile} ${colors.dim('(default)')}` : profile}`, - `${profiles[profile].apiKey}`, - `${profiles[profile].baseUrl}` - ]) -}) +const profileList = Object.keys(profiles).map((profile) => [ + `${defaultProfile === profile ? `${profile} ${colors.dim('(default)')}` : profile}`, + `${profiles[profile].apiKey}`, + `${profiles[profile].baseUrl}` +]) ``` - [ ] **Apply this suggestion**
    Suggestion importance[1-10]: 7 Why: This suggestion improves maintainability and readability by reducing code duplication, which is beneficial but not as critical as fixing bugs or security issues.
    		7