Skip to content

Conversation

@andsimakov
Copy link
Contributor

@andsimakov andsimakov commented May 22, 2025

PR Type

Enhancement, Documentation


Description

  • Add code formatters Black and isort with configuration.

    • Update pyproject.toml with formatter dependencies and settings.
    • Add .editorconfig for consistent editor behavior.
    • Add .pre-commit-config.yaml for pre-commit integration.
  • Integrate formatting checks into CI pipeline.

    • Update GitHub Actions workflow to run Black and isort checks.
  • Provide documentation for pre-commit hook usage.

    • Add docs/pre_commit_hook.md with setup and usage instructions.

Changes walkthrough 📝

Relevant files
Enhancement
.editorconfig
Add .editorconfig for consistent editor settings                 

.editorconfig

  • Add editor configuration for consistent code style.
  • Specify indentation, charset, and line endings for various file types.
  • +35/-0   
    ci_pipeline.yml
    Add Black and isort checks to CI pipeline                               

    .github/workflows/ci_pipeline.yml

  • Integrate Black and isort checks into test job.
  • Minor formatting and whitespace adjustments.
  • +16/-5   
    .pre-commit-config.yaml
    Add pre-commit config for code formatters                               

    .pre-commit-config.yaml

  • Add pre-commit configuration for Black and isort.
  • Exclude specific directories from formatting checks.
  • +15/-0   
    pyproject.toml
    Add formatter dependencies and configuration                         

    pyproject.toml

  • Add Black, isort, and pre-commit as development dependencies.
  • Add configuration for Black and isort.
  • +44/-0   
    Documentation
    pre_commit_hook.md
    Document pre-commit hook usage and setup                                 

    docs/pre_commit_hook.md

  • Add documentation for setting up and using pre-commit hooks.
  • Provide usage examples and references.
  • +32/-0   

    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • @qodo-free-for-open-source-projects

    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

    Configuration Conflict

    The isort configuration in .editorconfig (lines 15-18) may conflict with the more detailed configuration in pyproject.toml. Consider removing the duplicate configuration from .editorconfig to avoid potential conflicts.

    line_length=120
    known_first_party=cover_agent,tests_integration
    multi_line_output=3
    default_section=THIRDPARTY
    Redundant Arguments

    The pre-commit hooks include "--check" and "--check-only" arguments which will prevent automatic formatting and only report issues. This might be confusing for developers who expect pre-commit to fix formatting issues automatically.

          args: ["--check"]
          exclude: ^(templated_tests|cover_agent/lsp_logic)/
    
    - repo: https://github.com/pycqa/isort
      rev: 6.0.1
      hooks:
        - id: isort
          args: ["--check-only", "--diff"]

    @qodo-free-for-open-source-projects
    Copy link

    qodo-free-for-open-source-projects bot commented May 22, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    General
    Remove misplaced configuration settings

    The settings in the .editorconfig for Python files appear to be isort-specific
    configuration options that don't belong in .editorconfig. These should be moved
    to the appropriate configuration file (like pyproject.toml) where they're
    already defined.

    .editorconfig [14-18]

     [*.py]
    -line_length=120
    -known_first_party=cover_agent,tests_integration
    -multi_line_output=3
    -default_section=THIRDPARTY
    +indent_style = space
    +indent_size = 4
    • Apply / Chat
    Suggestion importance[1-10]: 7

    __

    Why: The suggestion correctly identifies that isort-specific options like line_length, known_first_party, multi_line_output, and default_section do not belong in .editorconfig and should be managed in pyproject.toml, improving configuration clarity and maintainability.

    Medium
    Enable auto-formatting in pre-commit

    The --check argument in the Black pre-commit hook will cause commits to fail
    without fixing the issues. For pre-commit hooks, it's better to let Black
    automatically format the code rather than just checking it.

    .pre-commit-config.yaml [2-8]

     - repo: https://github.com/psf/black
       rev: 25.1.0
       hooks:
         - id: black
           language_version: python3.12
    -      args: ["--check"]
           exclude: ^(templated_tests|cover_agent/lsp_logic)/
    • Apply / Chat
    Suggestion importance[1-10]: 6

    __

    Why: This suggestion would make the Black pre-commit hook auto-format code instead of just checking, which is a reasonable improvement for developer experience, but the current PR intentionally uses --check to enforce style without auto-modification, so the impact is moderate and context-dependent.

    Low
    Enable auto-fixing in pre-commit

    Similar to the Black hook, the isort hook is configured to only check and report
    issues without fixing them. For pre-commit hooks, it's better to let isort
    automatically fix import ordering.

    .pre-commit-config.yaml [10-15]

     - repo: https://github.com/pycqa/isort
       rev: 6.0.1
       hooks:
         - id: isort
    -      args: ["--check-only", "--diff"]
           exclude: ^(templated_tests|cover_agent/lsp_logic)/
    • Apply / Chat
    Suggestion importance[1-10]: 6

    __

    Why: The suggestion proposes letting isort auto-fix import order in pre-commit, which can streamline workflows, but the PR intentionally uses --check-only to enforce style without automatic changes, so while the suggestion is valid, its importance is moderate and context-dependent.

    Low
    Learned
    best practice
    Improve comment documentation clarity

    The commented URL at the top of the file should be properly formatted as a
    comment explaining the purpose of the file rather than appearing as
    commented-out code. Consider adding a more descriptive comment about
    EditorConfig's purpose.

    .editorconfig [1]

    -# http://editorconfig.org
    +# EditorConfig configuration file - http://editorconfig.org
    +# Defines consistent coding styles across different editors and IDEs
    • Apply / Chat
    Suggestion importance[1-10]: 6

    __

    Why:
    Relevant best practice - Remove or uncomment commented-out code that has been replaced or is no longer needed to improve code readability and maintainability.

    Low
    • Update

    @andsimakov andsimakov force-pushed the feature/333-add-code-formatters branch from 915e0c0 to e5a77f2 Compare May 22, 2025 16:32
    @andsimakov andsimakov force-pushed the feature/333-add-code-formatters branch from e5a77f2 to ef10932 Compare May 22, 2025 16:38
    @EmbeddedDevops1
    Copy link
    Collaborator

    EmbeddedDevops1 commented May 22, 2025

    @andsimakov Looks good. Two requests:

    1. Any chance you can add the pre-commit command to the Makefile as well?
    2. Also please add those linter checks to a new Makefile target called "lint-check" and use that command in CI to be consistent.

    The idea is to have CI run as many Makefile commands as possible so we can have consistency between our local environment and CI.

    @andsimakov andsimakov force-pushed the feature/333-add-code-formatters branch 4 times, most recently from 5989417 to 9676957 Compare May 23, 2025 13:15
    @andsimakov andsimakov force-pushed the feature/333-add-code-formatters branch from 9676957 to e55b517 Compare May 23, 2025 13:26
    @andsimakov andsimakov force-pushed the feature/333-add-code-formatters branch from 1051ebd to 51719f0 Compare May 23, 2025 14:48
    @andsimakov andsimakov force-pushed the feature/333-add-code-formatters branch from 51719f0 to be9a5b2 Compare May 23, 2025 14:59
    - name: Run E2E Docker tests
    env:
    OPENAI_API_KEY: "This_is_a_fake_API_key"
    OPENAI_API_KEY: ${{ env.FAKE_OPENAI_API_KEY }}
    Copy link
    Collaborator

    Choose a reason for hiding this comment

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

    @andsimakov where do you specify this env var?

    @EmbeddedDevops1 EmbeddedDevops1 linked an issue May 27, 2025 that may be closed by this pull request
    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.

    Add code formatters

    2 participants