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

fix: externalised json output as bool to represent unoplat pydantic s… #117

Merged
merged 1 commit into from
Sep 7, 2024

Conversation

JayGhiya
Copy link
Member

@JayGhiya JayGhiya commented Sep 7, 2024

User description

…chema in json


PR Type

enhancement


Description

  • Introduced a new boolean flag json_output in the AppConfig class to control JSON output.
  • Enhanced the codebase_summary module to conditionally write JSON output to a file based on the json_output flag.
  • JSON output file is named using the codebase_name and a timestamp for uniqueness.

Changes walkthrough 📝

Relevant files
Enhancement
external_config.py
Add `json_output` boolean flag to AppConfig                           

unoplat-code-confluence/unoplat_code_confluence/configuration/external_config.py

  • Added a new boolean field json_output to AppConfig.
  • Default value for json_output is set to False.
  • +1/-0     
    codebase_summary.py
    Implement conditional JSON output with timestamped filename

    unoplat-code-confluence/unoplat_code_confluence/summary_parser/codebase_summary.py

  • Imported datetime module.
  • Added json_output and codebase_name attributes to the class.
  • Conditional JSON file writing based on json_output flag.
  • JSON file name includes a timestamp and codebase_name.
  • +12/-6   

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

    @JayGhiya JayGhiya added the bug Something isn't working label Sep 7, 2024
    @JayGhiya JayGhiya self-assigned this Sep 7, 2024
    @JayGhiya JayGhiya linked an issue Sep 7, 2024 that may be closed by this pull request
    @codiumai-pr-agent-pro codiumai-pr-agent-pro bot added the enhancement New feature or request label Sep 7, 2024
    Copy link

    PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

    PR Reviewer Guide 🔍

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

    File Naming
    The JSON output file is created with a dynamic name using timestamp, but there's no mechanism to ensure uniqueness if multiple files are created in the same second.

    Copy link

    PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Maintainability
    Use a more descriptive variable name for the exception object

    Consider using a more descriptive variable name instead of 'e' for the exception
    object to improve code readability.

    unoplat-code-confluence/unoplat_code_confluence/summary_parser/codebase_summary.py [99-103]

    -except Exception as e:
    -    logger.error(f"Error generating codebase summary: {e}")
    +except Exception as error:
    +    logger.error(f"Error generating codebase summary: {error}")
         logger.exception("Traceback:")
         sys.exit(1)
     
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Using a more descriptive variable name for the exception object enhances code readability and maintainability, making it easier for other developers to understand the code.

    7
    Best practice
    Use json.dump() for writing JSON data to a file with proper formatting

    Consider using a context manager (with statement) when opening the file for writing.
    This ensures that the file is properly closed after writing, even if an exception
    occurs.

    unoplat-code-confluence/unoplat_code_confluence/summary_parser/codebase_summary.py [114-115]

     with open(f"{self.codebase_name}_{current_timestamp}.json", "w") as f:
    -    f.write(json_unoplat_codebase_summary)
    +    json.dump(unoplat_codebase_summary.model_dump(), f, indent=2)
     
    • Apply this suggestion
    Suggestion importance[1-10]: 5

    Why: The suggestion to use json.dump() with proper formatting is valid and improves the readability of the JSON output. However, the existing code already uses a context manager, so the suggestion does not address a critical issue.

    5

    @JayGhiya JayGhiya merged commit c3652d2 into main Sep 7, 2024
    1 check passed
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    bug Something isn't working enhancement New feature or request Review effort [1-5]: 2
    Projects
    Status: Done
    Development

    Successfully merging this pull request may close these issues.

    Remove json output as of now and make it a flag
    1 participant