-
Notifications
You must be signed in to change notification settings - Fork 745
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
Adding Estimated Review Effort Feature and Handling Cases with No Detected Language #306
Conversation
PR Analysis
PR Feedback
|
/improve --extended |
if self.git_provider.is_supported("gfm_markdown"): | ||
pr_body += "<details> <summary>files:</summary>\n\n" | ||
for file in value: | ||
filename = file['filename'].replace("'", "`") | ||
description = file['changes in file'] | ||
pr_body += f'`{filename}`: {description}\n' | ||
if self.git_provider.is_supported("gfm_markdown"): | ||
pr_body +="</details>\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: Consider using a context manager to handle the opening and closing of the details tag.
if self.git_provider.is_supported("gfm_markdown"): | |
pr_body += "<details> <summary>files:</summary>\n\n" | |
for file in value: | |
filename = file['filename'].replace("'", "`") | |
description = file['changes in file'] | |
pr_body += f'`{filename}`: {description}\n' | |
if self.git_provider.is_supported("gfm_markdown"): | |
pr_body +="</details>\n" | |
if self.git_provider.is_supported("gfm_markdown"): | |
pr_body += "<details> <summary>files:</summary>\n\n" | |
for file in value: | |
filename = file['filename'].replace("'", "`") | |
description = file['changes in file'] | |
pr_body += f'`{filename}`: {description}\n' | |
pr_body +="</details>\n" |
"require_tests": get_settings().pr_reviewer.require_tests_review, | ||
"require_security": get_settings().pr_reviewer.require_security_review, | ||
"require_focused": get_settings().pr_reviewer.require_focused_review, | ||
"require_estimate_effort_to_review": get_settings().pr_reviewer.require_estimate_effort_to_review, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: Consider using a dictionary comprehension to make the code more readable and concise.
"require_tests": get_settings().pr_reviewer.require_tests_review, | |
"require_security": get_settings().pr_reviewer.require_security_review, | |
"require_focused": get_settings().pr_reviewer.require_focused_review, | |
"require_estimate_effort_to_review": get_settings().pr_reviewer.require_estimate_effort_to_review, | |
requirements = {req: getattr(get_settings().pr_reviewer, req) for req in ['require_tests', 'require_security', 'require_focused', 'require_estimate_effort_to_review']} |
Estimated effort to review [1-5]: | ||
type: string | ||
description: >- | ||
Estimate, on a scale of 1-5 (inclusive), the time and effort required to review this PR by an experienced and knowledgeable developer. 1 means short and easy review , 5 means long and hard review. | ||
Take into account the size, complexity, quality, and the needed changes of the PR code diff. | ||
Explain your answer shortly (1-2 sentences). | ||
{%- endif %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: Consider using a numeric type for the "Estimated effort to review [1-5]" input instead of a string.
Estimated effort to review [1-5]: | |
type: string | |
description: >- | |
Estimate, on a scale of 1-5 (inclusive), the time and effort required to review this PR by an experienced and knowledgeable developer. 1 means short and easy review , 5 means long and hard review. | |
Take into account the size, complexity, quality, and the needed changes of the PR code diff. | |
Explain your answer shortly (1-2 sentences). | |
{%- endif %} | |
Estimated effort to review [1-5]: | |
type: number | |
description: >- | |
Estimate, on a scale of 1-5 (inclusive), the time and effort required to review this PR by an experienced and knowledgeable developer. 1 means short and easy review , 5 means long and hard review. | |
Take into account the size, complexity, quality, and the needed changes of the PR code diff. | |
Explain your answer shortly (1-2 sentences). |
@@ -42,6 +42,7 @@ def convert_to_markdown(output_data: dict, gfm_supported: bool=True) -> str: | |||
"General suggestions": "💡", | |||
"Insights from user's answers": "📝", | |||
"Code feedback": "🤖", | |||
"Estimated effort to review [1-5]": "⏱️", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: Consider using a more descriptive key for the "Estimated effort to review [1-5]" feature.
"Estimated effort to review [1-5]": "⏱️", | |
"Estimated Review Effort (1-5)": "⏱️", |
@@ -42,6 +42,11 @@ def sort_files_by_main_languages(languages: Dict, files: list): | |||
files_sorted = [] | |||
rest_files = {} | |||
|
|||
# if no languages detected, put all files in the "Other" category | |||
if not languages: | |||
files_sorted = [({"language": "Other", "files": list(files_filtered)})] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: Consider using a more Pythonic way to create a list with a single dictionary.
files_sorted = [({"language": "Other", "files": list(files_filtered)})] | |
files_sorted = [{"language": "Other", "files": list(files_filtered)}] |
Adding Estimated Review Effort Feature and Handling Cases with No Detected Language
PR Type:
Enhancement
PR Description:
This PR introduces two main enhancements. The first one is the addition of an "Estimated effort to review" feature, which estimates the time and effort required to review a PR on a scale of 1-5. The second enhancement is the handling of cases where no language is detected in a PR. In such cases, all files are categorized under "Other". Additionally, the PR includes minor adjustments to the markdown conversion and test fixes.
PR Main Files Walkthrough:
files:
pr_agent/algo/language_handler.py
: Added a condition to handle cases where no languages are detected. In such cases, all files are categorized under "Other".pr_agent/algo/utils.py
: Added a new markdown icon for the "Estimated effort to review" feature.pr_agent/git_providers/git_provider.py
: Added a condition to handle cases where no languages are detected, logging an info message in such cases.pr_agent/tools/pr_description.py
: Added support for GitHub Flavored Markdown (GFM) to the PR answer preparation. If GFM is supported, file details are wrapped in a collapsible details tag.pr_agent/tools/pr_reviewer.py
: Added a new requirement for estimating the effort to review a PR.tests/unittest/test_language_handler.py
: Updated the expected output of the test for the case where no languages are detected to include the actual files.pr_agent/settings/configuration.toml
: Added a new configuration option to require an estimate of the effort to review a PR.pr_agent/settings/pr_reviewer_prompts.toml
: Added a new prompt for estimating the effort to review a PR on a scale of 1-5.