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

Fixed error logging in Azure DevOps provider #900

Closed
wants to merge 2 commits into from

Conversation

taita2104
Copy link

@taita2104 taita2104 commented May 13, 2024

User description

Fixed a bug that caused "%s" to be logged when errors occurred during file diffs using in Azure DevOps provider


PR Type

Bug fix


Description

  • Fixed incorrect string formatting in error logging within the Azure DevOps provider. This change ensures that the file name and version are properly displayed in the log when an error occurs during file content retrieval.

Changes walkthrough 📝

Relevant files
Bug fix
azuredevops_provider.py
Fix Error Logging String Formatting in Azure DevOps Provider

pr_agent/git_providers/azuredevops_provider.py

  • Corrected the string formatting in error logging for both new and
    original file content retrieval failures.
  • +6/-6     

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

    Copy link

    PR Description updated to latest commit (c137c9a)

    Copy link

    PR Review 🔍

    ⏱️ Estimated effort to review [1-5]

    2, because the changes are straightforward and limited to error logging string formatting. The context and the scope of the changes are clear, making it easier to review.

    🏅 Score

    90

    🧪 Relevant tests

    No

    ⚡ Possible issues

    No

    🔒 Security concerns

    No

    Copy link

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Enhancement
    Replace old string formatting with f-string

    Replace the string formatting using '%' with the more modern f-string for better
    readability and performance.

    pr_agent/git_providers/azuredevops_provider.py [277-280]

    -"Failed to retrieve new file content of %s at version %s. Error: %s" %
    -(file,
    -version,
    -str(error)),
    +f"Failed to retrieve new file content of {file} at version {version}. Error: {error}"
     
    Suggestion importance[1-10]: 8

    Why: The suggestion correctly identifies an improvement in string formatting from '%' to f-string, which enhances readability and performance. This change is applicable to the new code added in the PR.

    8

    @mrT23
    Copy link
    Collaborator

    mrT23 commented May 14, 2024

    @taita2104 if we are already fixing this (i admit i saw it yesterday, and was too lazy to fix), maybe follow the code suggestion ? it seems a more modern approach

    image

    @mrT23
    Copy link
    Collaborator

    mrT23 commented May 15, 2024

    @taita2104
    thanks for the PR.
    the root cause for the issue was incorrect fetching of files, and this, I believe, fixed it
    #902

    so I am closing this PR. i also fixed in #902 the logging

    @mrT23 mrT23 closed this May 15, 2024
    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.

    None yet

    2 participants