Skip to content
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

refactor(cli): Correct message on listing projects #624

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

mrswastik-robot
Copy link

@mrswastik-robot mrswastik-robot commented Jan 12, 2025

User description

Description

This PR corrects the invalid message displayed while listing projects with cmd - keyshade project list my-workspace

Fixes #621

Screenshots of relevant screens

Change is done on the list.projects.ts file:

image

Developer's checklist

  • My PR follows the style guidelines of this project
  • I have performed a self-check on my work

If changes are made in the code:

  • I have followed the coding guidelines
  • My changes in code generate no new warnings
  • My changes are breaking another fix/feature of the project
  • I have added test cases to show that my feature works
  • I have added relevant screenshots in my PR
  • There are no UI/UX issues

Documentation Update

  • This PR requires an update to the documentation at docs.keyshade.xyz
  • I have made the necessary updates to the documentation, or no documentation changes are required.

PR Type

Bug fix


Description

  • Corrected the error message when no projects are found.

  • Updated the CLI output for better clarity.


Changes walkthrough 📝

Relevant files
Bug fix
list.project.ts
Corrected CLI message for empty project list                         

apps/cli/src/commands/project/list.project.ts

  • Updated the error message from "No forks found" to "No projects
    found".
  • Ensured the correct message is displayed when no projects are listed.
  • +1/-1     

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis ✅

    621 - Fully compliant

    Compliant requirements:

    • Fix incorrect error message when listing projects
    • Message should display "No projects found" instead of current message
    ⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ No major issues detected

    Copy link
    Contributor

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Fix array length check inconsistency to prevent potential undefined behavior

    The code uses projects.length to check array size but iterates over data.items. This
    inconsistency could cause bugs. Use data.items.length instead for the length check.

    apps/cli/src/commands/project/list.project.ts [41-44]

    -if (projects.length > 0) {
    +if (data.items.length > 0) {
       data.items.forEach((project: any) => {
         Logger.info(`- ${project.name} (${project.slug})`)
       })
     }
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: The inconsistency between using 'projects.length' for the check but 'data.items' for iteration could lead to runtime errors if these arrays are different. This is a significant bug risk that should be addressed.

    8
    General
    Improve type safety by replacing any type with proper interface definition

    Replace the any type with a proper interface or type definition for the project
    object to ensure type safety.

    apps/cli/src/commands/project/list.project.ts [42-43]

    -data.items.forEach((project: any) => {
    +interface Project {
    +  name: string;
    +  slug: string;
    +}
    +data.items.forEach((project: Project) => {
       Logger.info(`- ${project.name} (${project.slug})`)
     })
    • Apply this suggestion
    Suggestion importance[1-10]: 4

    Why: While adding type definitions improves code maintainability and catches potential type-related bugs early, this suggestion is primarily about code quality rather than fixing a critical issue.

    4

    @mrswastik-robot mrswastik-robot changed the title refactor(cli):Correct message on listing projects refactor(cli): Correct message on listing projects Jan 12, 2025
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    CLI: Incorrect message on listing projects
    1 participant