-
Notifications
You must be signed in to change notification settings - Fork 52
Pull Request Guidelines
This document defines the team conventions for creating, reviewing, and merging pull requests on the Mithril project. It complements the general Contributing Guidelines and focuses on the day-to-day workflow that every Core team member is expected to follow.
-
Name your branch
<github_handle>/<ticket-number>-description-name-for-pr— always include the ticket number for traceability - Only open a pull request when it is ready for review — the only exceptions are when CI testing is needed or during a pairing session
- Mark unfinished pull requests as draft — never ask reviewers to look at a moving target
- Self-review your code and ensure the CI is green before requesting a review — a red CI means the pull request is not ready
- Write a clean, focused description with bold and backticks — a well-presented pull request sets the tone for a productive review
-
Use
Closes #123orRelates to #123and assign yourself — link every pull request to an issue and make ownership visible - Treat Copilot as a real reviewer — acknowledge every comment as fixed, false positive, or disagreed
- Be polite, comment only when necessary, and escalate disagreements to a call — no comment is better than a useless one
- Review pull requests daily as a high priority — prompt reviewing is a matter of reciprocity and team spirit
- Ask for help on chat channels, not through a review request — share the branch and ask specific questions instead of submitting incomplete work for formal review
- Branch Naming
- When to Create a Pull Request
- Draft Status
- Pull Request Readiness
- Pull Request Description
- Issue Linking and Assignees
- Copilot Review
- Code Review Conduct
- Resolving Conversations
- Reviewer Responsibilities
- Review Priority
- Review Approvals
- External Contributors
- Asking for Help
Branches must follow the pattern: <github_handle>/<ticket-number>-description-name-for-pr
Including the ticket number ensures traceability between branches and issues. All branches are visible to the team, so there is no need to rush into creating a pull request — prepare and review the work on the branch first.
A pull request should only be created when it falls into one of these situations:
- The code is ready for review — this is the primary reason
- CI testing is needed — when the continuous integration pipeline must run against the changes
- Pairing session — when collaborating with another developer on the same branch
Do not create a pull request simply to make the branch more visible. Branches are already accessible to the entire team.
Any pull request that is not ready for review must be marked as a draft.
Marking a pull request as draft serves a critical purpose: it prevents reviewers from spending time on code that is still changing. Reviewing a moving target is inefficient and leads to fatigue, which in turn causes reviewers to miss important details.
There is an important distinction to maintain:
| Situation | Action |
|---|---|
| Need help with the implementation | Share the branch or a draft pull request and ask on Slack |
| Code is complete and ready for review | Open (or un-draft) the pull request and request reviewers |
Before opening a pull request for review (or converting a draft to ready), the developer must:
- Self-review the diff — read through every change as if reviewing someone else's code
- Ensure the CI is green — a pull request with a red CI should not be submitted for review except in exceptional circumstances
- Address Copilot feedback — Copilot performs the first round of review and handles minor nitpicks, freeing human reviewers to focus on what matters most
A good description strikes a balance between too short and too long. It should be summarized and focused, giving reviewers enough context to understand the why and the what without overwhelming them.
Formatting best practices:
- Use bold to highlight key concepts or sections
- Use
`backticks`for code snippets, file names, configuration values, and commands - Keep the description concise — avoid walls of text
The presentation quality of a pull request description matters. Given the volume of reviews the team handles, a clean and well-structured description reduces cognitive load and sets a positive tone for the review.
Use the appropriate keyword in the pull request description:
| Keyword | Behavior |
|---|---|
Closes #123 |
Merging the pull request automatically closes the linked issue and moves it to done on the board |
Relates to #123 |
Creates a reference without closing the issue upon merge |
Always assign the pull request to yourself. This simplifies tracking and allows reviewers to quickly identify who is responsible for which work.
Copilot should be treated as a real reviewer. Its comments deserve the same attention as those from a human colleague.
When addressing a Copilot comment:
- Fixed — indicate the comment was valid and has been addressed
- False positive — explain why the suggestion does not apply
- Disagree — provide reasoning for the alternative approach
For simple nitpick fixes (typos, formatting), a thumbs-up emoji followed by resolving the conversation is sufficient to indicate the fix was applied.
Never close Copilot comments without acknowledgment — doing so removes context that other reviewers may need.
- Be polite and kind — even when frustrated, maintain a constructive tone
- Comment only when necessary — focus on meaningful feedback: small optimizations or significant problems
- Do not comment to fill the void — no comment is better than a useless one
- Escalate complex disagreements to a call — when a back-and-forth on a comment grows too long, switch to a synchronous discussion to find a solution or an agreed-upon trade-off
- Respond to every comment, even if just to acknowledge it
- Do not take feedback personally — reviews are about the code, not the person
- Simple one-round conversations (e.g., a typo fix): resolve immediately after the fix is applied to reduce noise
- Multi-round conversations: leave open until the pull request is ready to merge
- All conversations must be resolved before merging
- The author resolves conversations once the required changes are made and the reviewer has approved
A review is not just a diff scan. Reviewers must conduct a full review of the code, both during the initial pass and after the author addresses comments. This ensures correctness beyond just the changed lines.
Force-pushing after a rebase can make prior review comments outdated, which forces the reviewer to start over. For long-running pull requests (one to two weeks), developers should rebase on the main branch regularly to minimize future merge conflicts and reduce the blast radius of a single rebase.
Pull requests that are marked as "in review" are a high priority and must be addressed daily.
Prompt reviewing is expected as a matter of reciprocity and team spirit. If you expect fast reviews on your own pull requests, extend the same courtesy to your colleagues. Reviewing is an essential part of the job, regardless of the time a complex pull request may require.
If a review seems to be stalling, a gentle nudge in the team chat channel is appropriate.
Given the small size of the team, approval is requested from everyone. This ensures:
- Consistent code quality across the project
- Every team member stays aware of the changes being introduced
In larger teams, limiting approvals to two or three individuals would be appropriate.
Avoid using "Request Changes" unless something is critically wrong — requesting changes blocks the pull request from merging if the reviewer becomes unavailable.
The chat channels are the place to ask for help. Keep discussions organized within a single thread.
The team is always available to prevent developers from getting stuck, but there is an important boundary:
| Need | Approach |
|---|---|
| Stuck on implementation | Share the branch or a draft pull request and ask specific questions on chat channels |
| Ready for formal review | Open the pull request, ensure it is clean, self-reviewed, and nearly complete |
A formal review request is not a substitute for asking for help. If the work is not complete, share the branch and request targeted assistance instead.