Require PRs to have description #27053
Conversation
Add CI check that validates PR descriptions are not empty. The check handles both cases: PRs using the template and PRs without template. It ignores Sourcery AI bot summaries and requires minimum meaningful content. Includes validation script, GitHub workflow, and test suite.
Reviewer's GuideThis PR enforces non-empty, meaningful PR descriptions by adding a Python validation script, a test suite, and a GitHub Actions workflow to run the checks on pull request events, handling both templated and free-form descriptions while ignoring Sourcery AI summaries. Sequence diagram for PR description validation on pull requestsequenceDiagram
participant GitHub
participant Workflow
participant Script
GitHub->>Workflow: Trigger on PR event
Workflow->>Workflow: Checkout repo, setup Python, run tests
Workflow->>Workflow: Extract PR description to pr_description.txt
Workflow->>Script: Run validate-pr-description.py with pr_description.txt
Script->>Script: Validate description (template, length, ignore Sourcery)
Script-->>Workflow: Return pass/fail result
Workflow-->>GitHub: Mark check as passed/failed
Class diagram for PR description validation scriptclassDiagram
class validate_pr_description.py {
+remove_html_comments(text)
+remove_sourcery_section(text)
+extract_description_section(text)
+get_meaningful_content(text)
+validate_pr_description(description)
+main()
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Consider extracting the hard-coded minimum description lengths (20 and 50) into named constants or configuration so you can tweak them without changing validation logic.
- To improve maintainability and ease of adding edge cases, think about migrating the shell-based test suite to a Python test framework (like pytest) instead of a custom bash runner.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider extracting the hard-coded minimum description lengths (20 and 50) into named constants or configuration so you can tweak them without changing validation logic.
- To improve maintainability and ease of adding edge cases, think about migrating the shell-based test suite to a Python test framework (like pytest) instead of a custom bash runner.
## Individual Comments
### Comment 1
<location> `.github/bin/validate-pr-description.py:119-121` </location>
<code_context>
+ except FileNotFoundError:
+ print(f"Error: File '{description_file}' not found.", file=sys.stderr)
+ sys.exit(1)
+ except Exception as e:
+ print(f"Error reading file: {e}", file=sys.stderr)
+ sys.exit(1)
</code_context>
<issue_to_address>
**suggestion:** Generic exception handling may obscure specific errors.
Handle only expected exceptions, such as IOError, and log or re-raise unexpected ones to aid debugging.
```suggestion
except IOError as e:
print(f"IOError reading file '{description_file}': {e}", file=sys.stderr)
sys.exit(1)
except Exception as e:
print(f"Unexpected error reading file '{description_file}': {e}", file=sys.stderr)
raise
```
</issue_to_address>
### Comment 2
<location> `.github/bin/validate-pr-description.py:33-36` </location>
<code_context>
def extract_description_section(text):
"""
Extract content from the Description section of the template.
Returns None if template is not used.
"""
# Look for "## Description" heading
match = re.search(r'##\s+Description\s*\n(.*?)(?=##|$)', text, flags=re.DOTALL)
if match:
return match.group(1)
return None
</code_context>
<issue_to_address>
**suggestion (code-quality):** We've found these issues:
- Use named expression to simplify assignment and conditional ([`use-named-expression`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/use-named-expression/))
- Replace m.group(x) with m[x] for re.Match objects ([`use-getitem-for-re-match-groups`](https://docs.sourcery.ai/Reference/Default-Rules/suggestions/use-getitem-for-re-match-groups/))
```suggestion
if match := re.search(
r'##\s+Description\s*\n(.*?)(?=##|$)', text, flags=re.DOTALL
):
return match[1]
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull Request Overview
This PR introduces automated validation for pull request descriptions to ensure they contain meaningful content. The implementation includes a Python validation script, a comprehensive shell-based test suite, and a GitHub Actions workflow that runs on PR events (opened, edited, synchronize, reopened).
Key changes:
- Added validation logic that distinguishes between templated and non-templated PRs, requiring minimum content length while filtering out automated bot summaries
- Created a CI workflow that both tests the validator and enforces description requirements on actual PRs
- Implemented a test harness with fixtures covering various PR description scenarios
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
Is this really solving a problem ? |
This is true. But people often reply as a comment while they should also fill the PR description.
Yes. And that's what people might end up doing. |
2 participants
Add CI check that validates PR descriptions are not empty. The check handles both cases: PRs using the template and PRs without template. It ignores Sourcery AI bot summaries and requires minimum meaningful content.
Includes validation script, GitHub workflow, and test suite.
Summary by Sourcery
Enforce meaningful pull request descriptions by adding a validation script, accompanying tests, and a CI workflow to run the checks on PR events
New Features:
CI:
Tests: