Skip to content

Conversation

@KRRT7
Copy link
Contributor

@KRRT7 KRRT7 commented May 28, 2025

PR Type

Bug fix


Description

  • Use user-provided path directly for module root

  • Remove automatic prepending of current directory

  • Exit if no custom path provided


Changes walkthrough 📝

Relevant files
Bug fix
cmd_init.py
Direct use of custom module path                                                 

codeflash/cli_cmds/cmd_init.py

  • Changed module_root assignment for custom input
  • Removed Path(curdir) concatenation
  • Directly use absolute path from user
  • +1/-1     

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • @github-actions
    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Type inconsistency

    The module_root variable is assigned a Path for custom paths but remains a string in other branches, which may lead to unexpected type errors. Consider unifying the type across all cases.

    module_root = Path(custom_module_root_answer["path"])
    Path normalization

    The custom path is used directly and may remain relative; you may need to call .resolve() or validate its absolute form to avoid issues with relative paths.

    module_root = Path(custom_module_root_answer["path"])

    @github-actions
    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Normalize and validate module root

    Normalize the custom path and ensure it points to an existing directory before
    proceeding. Use expanduser() and resolve() to handle tilde or relative inputs and
    then check is_dir(), calling apologize_and_exit() on failure.

    codeflash/cli_cmds/cmd_init.py [204]

    -module_root = Path(custom_module_root_answer["path"])
    +module_root = Path(custom_module_root_answer["path"]).expanduser().resolve()
    +if not module_root.is_dir():
    +    apologize_and_exit(f"Provided module root '{module_root}' is not a directory.")
    Suggestion importance[1-10]: 8

    __

    Why: Using expanduser() and resolve() then checking is_dir() prevents invalid or non-existent paths at runtime, improving the robustness of module root selection.

    Medium

    Copy link
    Contributor

    @aseembits93 aseembits93 left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    some tests to check the behavior?

    @KRRT7 KRRT7 enabled auto-merge May 28, 2025 18:52
    Copy link
    Contributor

    @aseembits93 aseembits93 left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    lgtm

    @KRRT7 KRRT7 merged commit e244b65 into main May 28, 2025
    17 checks passed
    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.

    4 participants