docs: update PR template with contributor guidelines and testing info#954
Open
fuleinist wants to merge 1 commit intomassgen:mainfrom
Open
docs: update PR template with contributor guidelines and testing info#954fuleinist wants to merge 1 commit intomassgen:mainfrom
fuleinist wants to merge 1 commit intomassgen:mainfrom
Conversation
- Added checklist from CONTRIBUTING.md 'Before Submitting' section - Added PR Description section with What/Why/How/Testing/Screenshots - Added Testing section with example pytest commands - Added Documentation checkbox section - Added Review Process section with CodeRabbit commands - Improved formatting and organization Closes massgen#927
📝 WalkthroughWalkthroughUpdated the PR template file to restructure sections, replacing "Addressing Original Case Study" and "Additional Notes" sections with new "Testing," "Documentation," and "Review Process" sections to better guide contributors through the submission process. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 6 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/PULL_REQUEST_TEMPLATE/case_study_resolution.md:
- Line 26: Replace the hardcoded checklist item "PR targets `dev/v0.1.57` branch
(or `main` if dev branch doesn't exist yet)" with a generic, maintainable
reference such as "PR targets `dev/*` branch (or `main` if a dev branch doesn't
exist)" or "PR targets the current development branch (e.g., `dev/*`)"; update
the checklist text in the same template line so future releases don't require
manual edits.
- Around line 1-88: The PR template file case_study_resolution.md in
.github/PULL_REQUEST_TEMPLATE is misnamed for its generic content; either rename
the file to a descriptive generic name (e.g., detailed_change_summary.md) or
make it truly case-study-specific by adding sections like "Use Case Overview",
"Real-World Example", and "Configuration and Results" to
case_study_resolution.md, and then update CONTRIBUTING.md to document when to
choose this template versus the default (add a short "Which PR template to use"
section referencing the new filename and the case-study-template.md format in
docs/source/examples/case_studies/). Ensure the chosen fix updates references
and examples in CONTRIBUTING.md and that the template filename and headings
match the documented guidance.
Comment on lines
+1
to
+88
| ## 📝 PR Description | ||
|
|
||
| ### What | ||
| Brief description of changes | ||
|
|
||
| ### Why | ||
| Motivation and context | ||
|
|
||
| ### How | ||
| Technical approach taken | ||
|
|
||
| ### Testing | ||
| How you tested the changes | ||
|
|
||
| ### Screenshots | ||
| If UI changes (if applicable) | ||
|
|
||
| --- | ||
|
|
||
| ## ✅ Checklist (Before Submitting) | ||
|
|
||
| - [ ] Code passes all pre-commit hooks | ||
| - [ ] Tests pass locally | ||
| - [ ] Documentation is updated if needed | ||
| - [ ] Commit messages follow convention | ||
| - [ ] PR targets `dev/v0.1.57` branch (or `main` if dev branch doesn't exist yet) | ||
|
|
||
| --- | ||
|
|
||
| ## 🔄 Addressing Original Issue | ||
| *🔗 Issue Addressed: Closes #[issue-number]* | ||
|
|
||
| Explain how the changes made address the issues and requirements outlined in the original issue. | ||
|
|
||
| --- | ||
|
|
||
| ## 📋 Additional Notes | ||
| *Add any additional context, caveats, or follow-up items here* | ||
|
|
||
| --- | ||
|
|
||
| ## 💻 CODE CHANGES | ||
|
|
||
| ## Summary | ||
| Brief description of what was implemented to resolve the case study. | ||
| Brief description of what was implemented. | ||
|
|
||
| ## Key Changes | ||
| Describe the key changes made to the codebase, including new features, modifications, bug fixes, refactoring, or any other relevant updates. | ||
|
|
||
| --- | ||
|
|
||
| ## 🔄 Addressing Original Case Study | ||
| *🔗 Issue Addressed: Closes #[issue-number]* | ||
| ## 🧪 Testing | ||
| *Describe how you tested your changes. Include test commands if applicable.* | ||
|
|
||
| Example: | ||
| ```bash | ||
| # Run tests | ||
| uv run pytest massgen/tests/ -v | ||
|
|
||
| Explain how the changes made address the issues and requirements outlined in the original case study. Reference specific sections of the case study as needed. | ||
| # Run specific test file | ||
| uv run pytest massgen/tests/test_config_validator.py -v | ||
|
|
||
| For this PR to be merged, you MUST complete the case study by running it with the new version of MassGen and documenting the results. Create a new markdown file following [the case study template](./docs/source/examples/case_studies/case-study-template.md) and link it below: | ||
| - [Link to the completed case study markdown file](./docs/source/examples/case_studies/[your-case-study-file].md) | ||
| # Run linting/formatting | ||
| uv run pre-commit run --all-files | ||
| ``` | ||
|
|
||
| --- | ||
|
|
||
| ## 📝 Additional Notes | ||
| *Add any additional context, caveats, or follow-up items here* | ||
| ## 📚 Documentation | ||
| *Is documentation updated?* | ||
|
|
||
| - [ ] Yes - documentation added/updated in `docs/` | ||
| - [ ] No documentation changes needed | ||
|
|
||
| --- | ||
|
|
||
| ## 🔍 Review Process | ||
|
|
||
| 1. Automated checks will run on your PR | ||
| 2. **CodeRabbit** will provide AI-powered code review comments | ||
| 3. Maintainers will review your code | ||
| 4. Address any feedback or requested changes | ||
| 5. Once approved, PR will be merged | ||
|
|
||
| **Useful CodeRabbit commands** (in PR comments): | ||
| - `@coderabbitai review` - Trigger incremental review | ||
| - `@coderabbitai resolve` - Mark all comments as resolved | ||
| - `@coderabbitai summary` - Regenerate PR summary |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check PR template structure and usage
echo "=== PR templates in repository ==="
fd . .github/PULL_REQUEST_TEMPLATE/ --type f 2>/dev/null || echo "Directory not found, searching for PR templates..."
echo -e "\n=== All PULL_REQUEST files in .github ==="
fd "PULL_REQUEST" .github/ --type f 2>/dev/null || true
echo -e "\n=== Check for README or CONTRIBUTING in .github ==="
ls -la .github/ | head -20
echo -e "\n=== Check for CONTRIBUTING.md in repo root ==="
[ -f CONTRIBUTING.md ] && cat CONTRIBUTING.md || echo "No CONTRIBUTING.md found"
echo -e "\n=== Check for docs about case studies ==="
fd "case.study" docs/ --type f 2>/dev/null | head -10 || true
echo -e "\n=== List all files in docs structure ==="
fd . docs/ --max-depth 2 --type f 2>/dev/null | grep -i "case\|template\|readme" | head -20 || trueRepository: massgen/MassGen
Length of output: 33061
🏁 Script executed:
#!/bin/bash
# Check default PR template and understand template structure
echo "=== Default PR template content ==="
cat -n .github/pull_request_template.md | head -50
echo -e "\n=== Check if templates are identical ==="
echo "Size comparison:"
wc -l .github/pull_request_template.md .github/PULL_REQUEST_TEMPLATE/case_study_resolution.md
echo -e "\n=== Check if there are any GitHub workflow files that reference templates ==="
find .github/workflows -name "*.yml" -o -name "*.yaml" | xargs grep -l "template\|PULL_REQUEST" 2>/dev/null || echo "No workflow references found"
echo -e "\n=== Check CONTRIBUTING.md for template guidance ==="
grep -n -i "pull.request.*template\|template.*pull.request\|case_study.*template\|case.*study.*pr\|pr.*case" CONTRIBUTING.md | head -20 || echo "No specific template guidance found"
echo -e "\n=== Check for any issue/discussion about templates ==="
find . -name "*.md" -type f | xargs grep -l "PULL_REQUEST_TEMPLATE\|pull_request_template" 2>/dev/null | head -5Repository: massgen/MassGen
Length of output: 2965
Template filename doesn't reflect its generic purpose; add documentation for template selection.
The template file case_study_resolution.md is positioned in GitHub's PULL_REQUEST_TEMPLATE directory, which lets contributors choose between named templates. However, the content is completely generic (PR Description, Code Changes, Testing, Documentation) with no case-study-specific sections. Additionally, CONTRIBUTING.md provides no guidance on when contributors should select this template versus the default one.
To resolve this mismatch:
- Rename the template to reflect its actual generic purpose (e.g.,
detailed_change_summary.md), or - Make it case-study-specific by adding sections relevant to case study submissions (e.g., "Use Case Overview", "Real-World Example", "Configuration and Results"), or
- Add guidance to CONTRIBUTING.md explaining when contributors should use this template vs. the default template
If this template is specifically for case study documentation PRs, update CONTRIBUTING.md to clarify that case study PRs should use this template and follow the case-study-template.md format in docs/source/examples/case_studies/.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/PULL_REQUEST_TEMPLATE/case_study_resolution.md around lines 1 - 88,
The PR template file case_study_resolution.md in .github/PULL_REQUEST_TEMPLATE
is misnamed for its generic content; either rename the file to a descriptive
generic name (e.g., detailed_change_summary.md) or make it truly
case-study-specific by adding sections like "Use Case Overview", "Real-World
Example", and "Configuration and Results" to case_study_resolution.md, and then
update CONTRIBUTING.md to document when to choose this template versus the
default (add a short "Which PR template to use" section referencing the new
filename and the case-study-template.md format in
docs/source/examples/case_studies/). Ensure the chosen fix updates references
and examples in CONTRIBUTING.md and that the template filename and headings
match the documented guidance.
| - [ ] Tests pass locally | ||
| - [ ] Documentation is updated if needed | ||
| - [ ] Commit messages follow convention | ||
| - [ ] PR targets `dev/v0.1.57` branch (or `main` if dev branch doesn't exist yet) |
There was a problem hiding this comment.
Hardcoded version number will become outdated.
The checklist references a specific branch dev/v0.1.57, which will become stale with each release. Consider using a generic reference instead to reduce maintenance burden.
📝 Proposed fix to use generic branch reference
-- [ ] PR targets `dev/v0.1.57` branch (or `main` if dev branch doesn't exist yet)
+- [ ] PR targets the appropriate dev branch (e.g., `dev/v0.1.x`) or `main` if dev branch doesn't exist yet📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - [ ] PR targets `dev/v0.1.57` branch (or `main` if dev branch doesn't exist yet) | |
| - [ ] PR targets the appropriate dev branch (e.g., `dev/v0.1.x`) or `main` if dev branch doesn't exist yet |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/PULL_REQUEST_TEMPLATE/case_study_resolution.md at line 26, Replace
the hardcoded checklist item "PR targets `dev/v0.1.57` branch (or `main` if dev
branch doesn't exist yet)" with a generic, maintainable reference such as "PR
targets `dev/*` branch (or `main` if a dev branch doesn't exist)" or "PR targets
the current development branch (e.g., `dev/*`)"; update the checklist text in
the same template line so future releases don't require manual edits.
fuleinist
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #927
Summary by CodeRabbit
Release Notes