Update the PR templates to have the new testing instructions #289
Conversation
WalkthroughThe pull request updates the pull request template markdown file to improve clarity, conciseness, and formatting. It revises branch naming instructions, issue linking, feature description, testing instructions, and reorganizes the checklist, while removing outdated or redundant sections. No code or exported entities are affected. Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
![windsurf-bot[bot]](https://avatars.githubusercontent.com/in/1066231?s=60&v=4){kind=small}
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed everything up to e5f899e in 1 minute and 36 seconds. Click for details.
- Reviewed
166lines of code in1files - Skipped
0files when reviewing. - Skipped posting
2draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. .github/pull_request_template.md:10
- Draft comment:
Consider using a clearer placeholder (e.g., 'Closes #123') instead of 'Closes #issue-number' to avoid confusion. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
2. .github/pull_request_template.md:74
- Draft comment:
Consider updating the placeholder commands ('just A' and 'just B') with more descriptive examples or remove them if not applicable. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_cRhgPfbUezeiCWvb
You can customize 
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (9)
.github/pull_request_template.md (9)
4-5: Clarify branch naming placeholders.The placeholder for the current branch (
<Replace with your branch name>) could be more prescriptive. Consider showing the recommended pattern directly, for example:Current branch: feature/<issue-number>-<description>This reduces ambiguity for authors.
8-10: Emphasize correct issue linking.Using a live placeholder like
Closes #<issue-number>in the template can help ensure the user replaces it correctly:Closes #<issue-number>This prevents forgetting to update the placeholder with an actual issue number.
14-14: Streamline the feature description example.The example inside the HTML comment is clear, but converting it into a bullet list enhances readability:
<!-- Example: - Adds user authentication via OAuth2, including login flow and profile creation -->
20-21: Standardize example formatting under Implementation Details.Ensure consistent indentation and list styling in the HTML comment:
<!-- Example: - Used Strategy pattern for multiple auth providers - Middleware for authentication checks -->
24-27: Hide scaffolding notes behind comments.The “Important Notes” section currently appears in the rendered PR body. Wrap these instructions in an HTML comment so they guide the author during editing without cluttering the final description.
50-50: Prefer markdown headings over bold emphasis.Lines using
**bold text**for subsection titles (e.g., “Common Testing Instructions”, “Run pre-commit checks…”, etc.) should use markdown headings (###or####) to improve accessibility and satisfy MD036 (no-emphasis-as-heading).Also applies to: 54-54, 58-58, 62-62, 66-66, 74-74
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
50-50: Emphasis used instead of a heading
null(MD036, no-emphasis-as-heading)
71-72: Clarify example installation command.The placeholder
just install-[tool-name]is useful; consider showing a concrete example (e.g.,just install-docker) to help authors replace it correctly.
75-76: Enhance command formatting for clarity.The inline code for
just Aandjust Bis fine. For consistency, consider listing each command in its own code fence or adding a brief description (e.g., what “A” and “B” simulate) so authors know when to use them.
92-92: Checklist consistency.The new checklist item “All tests pass locally and in CI” is clear. You may wrap “CI” in backticks or spell it out (“continuous integration”) for added clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/pull_request_template.md(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
.github/pull_request_template.md
50-50: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
54-54: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
62-62: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
🔇 Additional comments (6)
.github/pull_request_template.md (6)
29-32: Changes Made section is clear.The key changes list and example bullets are well-structured inside HTML comments, and the exposed placeholders are correctly positioned for authors to fill.
38-42: Testing Performed guidelines are helpful.The instructions for describing both automated and manual testing are correctly hidden in HTML comments, making it easy for authors to follow.
44-45: Expose placeholders for Testing Performed.The blank bullets are rightly exposed for authors to list their testing steps.
47-49: Rename and organize testing section.Renaming “Pre-commit and Testing Instructions” to “Testing Instructions” aligns with the PR summary and simplifies the template.
81-81: Documentation Updates placeholder is visible.The example instructions are hidden in an HTML comment, and the blank bullet is correctly exposed for authors to fill.
87-87: Expose placeholder for documentation updates.The single hyphen is properly exposed, guiding authors to list documentation changes.
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 49f7f35 in 1 minute and 4 seconds. Click for details.
- Reviewed
31lines of code in1files - Skipped
0files when reviewing. - Skipped posting
3draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. .github/pull_request_template.md:54
- Draft comment:
The new 'Install the package in editable mode with development dependencies' section (with 'just install-dev') is a helpful addition to ensure developers set up their environment correctly. Consider mentioning that 'just' must be installed locally if it isn't already. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. .github/pull_request_template.md:66
- Draft comment:
Replacing the duplicate install instruction with explicit linting instructions ('just lint') improves clarity. Ensure that the 'just lint' command is properly defined in your project. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. .github/pull_request_template.md:70
- Draft comment:
The addition of a code formatting step ('just format') is a useful reminder for style consistency. The instructions are clear and consistent with the rest of the template. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_6xocFiQH6f2gWsjA
You can customize
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (5)
.github/pull_request_template.md (5)
34-35: Remove empty list placeholders
Blank list items (-) may confuse contributors. Remove them or replace with a descriptive prompt such as- Describe the changes made.
44-45: Remove empty testing placeholders
Standalone dashes without context can be misleading. Either remove them or provide guidance on what to fill in.
56-73: Fix inline code fence formatting for commands
Inline fences likejust install-devshould be proper fenced code blocks with a language identifier. For example:just install-devApply this change to all command examples in this section.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
58-58: Emphasis used instead of a heading
null(MD036, no-emphasis-as-heading)
66-66: Emphasis used instead of a heading
null(MD036, no-emphasis-as-heading)
70-70: Emphasis used instead of a heading
null(MD036, no-emphasis-as-heading)
83-84: Use descriptive placeholders for example commands
Generic placeholders (just A,just B) offer little guidance. Consider realistic examples or parameterized placeholders (e.g.,just test-feature) to better illustrate usage.
95-96: Remove empty documentation placeholders
Empty list items (-) can be confusing. Remove them or replace with example documentation changes.
🧹 Nitpick comments (3)
.github/pull_request_template.md (3)
4-5: Consistent placeholder formatting
Use square bracket placeholders like[Replace with your branch name]to match the style used elsewhere in the template, rather than angle brackets.
24-27: Use a proper heading for Important Notes
To improve document structure and avoid relying on bold text, convert this into a markdown heading (e.g.,### Important Notes) instead of using emphasis.
50-50: Convert bold section titles to headings
Static analysis flags emphasis as headings (MD036). For better readability and semantic structure, use markdown headings (e.g.,### Common Testing Instructions,#### Install the package…) instead of bold text.Also applies to: 54-54, 58-58, 66-66, 70-70
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
50-50: Emphasis used instead of a heading
null(MD036, no-emphasis-as-heading)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/pull_request_template.md(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
.github/pull_request_template.md
50-50: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
54-54: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
58-58: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
66-66: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
70-70: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: continuous-integration (3.11)
- GitHub Check: continuous-integration (3.10)
🔇 Additional comments (7)
.github/pull_request_template.md (7)
8-10: Related issue placeholder
This section looks good.
14-14: Feature description example
The example is clear and concise.
20-21: Implementation details bullet list
Nice use of bullet points to outline the architectural decisions.
29-29: Changes Made comment
The prompt guiding where to list key changes is clear.
47-48: Testing Instructions header and comment
The heading and its comment are correctly structured for PR templates.
89-89: Documentation Updates header
The header is clear and consistent with the rest of the template.
100-100: Checklist entry for passing tests
Great addition ensuring that all tests pass both locally and in CI.
removed brackets so double click selects the whole name/word
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
.github/pull_request_template.md (2)
78-80: Duplicate: heading formatting
See the comment on lines 46-66 about replacing bold emphasis with proper headings.
4-5: 🛠️ Refactor suggestionStandardize placeholder formatting for branch name
The placeholder
BRANCH_NAMEcould be confused with literal text. Consider using square brackets or inline code to clearly mark it as a placeholder. For example:- Current branch: BRANCH_NAME + Current branch: [branch-name]
🧹 Nitpick comments (6)
.github/pull_request_template.md (6)
8-9: Use consistent placeholder style for related issueAlign the issue-number placeholder with the branch-name convention. For example:
- Closes #ISSUE_NUMBER + Closes #[issue-number]
18-19: Question: Keep or remove example implementation details?The commented example under Implementation Details may become outdated. Consider updating these bullets with real guidance or removing the snippet entirely to avoid confusion.
22-25: Prefer a proper heading instead of bold emphasis for Important NotesMarkdownlint flags emphasis used as a heading (MD036). You can switch to a level-3 heading for clarity:
- **Important Notes (please address or acknowledge):** + ### Important Notes (please address or acknowledge)
29-29: Clarify or remove the sample in "Changes Made"The commented example here may not reflect actual changes. Either replace it with real entries or remove the example block to keep the template clean.
Also applies to: 31-32
38-40: Clarify or remove the sample in "Testing Performed"Similarly, update or remove the commented example steps so they’re relevant and don’t clutter the section.
46-66: Use headings instead of bold emphasis for sub-section titlesSeveral sub-section titles use bold text. For better structure and to satisfy markdownlint, convert them to heading levels. For instance:
- **Common Testing Instructions** + #### Common Testing InstructionsRepeat similarly for the titles on lines 50, 54, 58, 62, and 66.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
46-46: Emphasis used instead of a heading
null(MD036, no-emphasis-as-heading)
50-50: Emphasis used instead of a heading
null(MD036, no-emphasis-as-heading)
54-54: Emphasis used instead of a heading
null(MD036, no-emphasis-as-heading)
62-62: Emphasis used instead of a heading
null(MD036, no-emphasis-as-heading)
66-66: Emphasis used instead of a heading
null(MD036, no-emphasis-as-heading)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/pull_request_template.md(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
.github/pull_request_template.md
46-46: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
50-50: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
54-54: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
62-62: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
66-66: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: continuous-integration (3.11)
- GitHub Check: continuous-integration (3.10)
🔇 Additional comments (3)
.github/pull_request_template.md (3)
43-44: Testing Instructions section is clear
The new heading and introductory comment clearly guide contributors on where to find the steps.
75-77: Skip
This example code block is already using a properbashfence and looks good.
94-94: Checklist addition is appropriate
The new checklist item aligns well with the testing requirements and RTC practices.
| ```just install-dev``` | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix inline code fence formatting for command snippets
Commands are currently wrapped inline (e.g., just test). It’s clearer to use fenced blocks with a language identifier. For example:
- ```just install-dev```
+ ```bash
+ just install-dev
+ ```Apply the same pattern to just pre-commit-run, just test, just lint, and just format.
Also applies to: 56-57, 60-61, 64-65, 68-69
🤖 Prompt for AI Agents
In .github/pull_request_template.md at lines 52-53 and also at lines 56-57,
60-61, 64-65, and 68-69, the commands are currently formatted as inline code
snippets. Change these to fenced code blocks with the language identifier "bash"
by replacing the inline backticks with triple backticks before and after the
command and adding "bash" after the opening backticks. This will improve
readability and clarity of the command snippets.
| ## Documentation Updates | ||
| <!-- Describe any documentation changes made or needed --> | ||
| <!-- Include updates to Sphinx docs, README, etc. --> | ||
| <!-- List any documentation changes made or needed --> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove placeholder dash in Documentation Updates
The lone - on line 89 serves as a confusing placeholder. Either populate the section with real updates or remove this line:
- -Also applies to: 85-87
🤖 Prompt for AI Agents
In .github/pull_request_template.md at lines 83 and also lines 85 to 87, remove
the placeholder dash '-' that currently serves as a confusing placeholder in the
Documentation Updates section. Either replace it with actual documentation
update details or delete the line entirely to avoid confusion.
| ## Checklist | ||
| <!-- Please verify each item by checking the box --> | ||
| - [ ] Branch name follows convention (`feature/description` or `feature/issue-number-description`) | ||
| - [ ] Testing added to pre-commit hook (`pre-commit run --all-files` passes) |
There was a problem hiding this comment.
This should probably use our just command
| ```bash | ||
| # These are basic defaults for testing. | ||
| # Please adjust as needed for complete testing of your changes | ||
| **Common Testing Instructions** |
There was a problem hiding this comment.
I would like to see groups of workflows of commands like:
Test github action
just some_command
just some_second_commandTest NAME_SUBJECT
just some_command- NAME A MANUAL STEP 1
- NAME MANUAL STEP 2
just some_second_command@husnain067 Please work with @Tatianah3 's proposal on what workflows have testing.
Feature Pull Request
Branch Naming Convention
Current branch:
feature/update-PR-template-testing-instructionsRelated Issue
Closes #286
Feature Description
Update the current PR templates with the new testing instructions used mostly in every PR, and remove the unused testing introductions and update them.
Implementation Details
pull-request_templateaccordingly to the new suggestions and feedback.Changes Made
pull-request_template.mdwith new changes.Testing Performed
Testing Instructions
pull-request_template.md.Documentation Updates
N/ABreaking Changes
Checklist
feature/descriptionorfeature/issue-number-description)pre-commit run --all-filespasses)pytest --covto verify)flake8or equivalent passes)Reviewer Notes
Important
Update
.github/pull_request_template.mdfor improved clarity, conciseness, and testing instructions..github/pull_request_template.mdfor clarity and conciseness.This description was created by
for 49f7f35. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit