Pull Request Guidelines - IntersectMBO/mithril GitHub Wiki

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.

The 12 Rules

  1. Name your branch <github_handle>/<ticket-number>-description-name-for-pr — always include the ticket number for traceability
  2. 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
  3. Mark unfinished pull requests as draft — never ask reviewers to look at a moving target
  4. Self-review your code and ensure the CI is green before requesting a review — a red CI means the pull request is not ready
  5. Write a clean, focused description with bold and backticks — a well-presented pull request sets the tone for a productive review
  6. Use Closes #123 or Relates to #123 and assign yourself — link every pull request to an issue and make ownership visible
  7. Treat Copilot as a real reviewer — acknowledge every comment as fixed, false positive, or disagreed
  8. Be polite, comment only when necessary, and escalate disagreements to a call — no comment is better than a useless one
  9. Review pull requests daily as a high priority — prompt reviewing is a matter of reciprocity and team spirit
  10. 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
  11. Keep every message short and reply in the comment thread: state what changed in one or two sentences, reviewer time is the scarcest resource of the project
  12. Own every line you submit: you are accountable for every line of the diff and every comment posted, never let an autonomous agent open a pull request, push commits or post comments without human intervention

Table of Contents

Branch Naming

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.

When to Create a Pull Request

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.

Draft Status

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

Pull Request Readiness

Before opening a pull request for review (or converting a draft to ready), the developer must:

  1. Self-review the diff — read through every change as if reviewing someone else's code
  2. Ensure the CI is green — a pull request with a red CI should not be submitted for review except in exceptional circumstances
  3. Address Copilot feedback — Copilot performs the first round of review and handles minor nitpicks, freeing human reviewers to focus on what matters most
  4. Fill the pull request template truthfully: tick a checklist item only when it holds for the current head commit, for example the CI box only once the CI is green on that commit
  5. Keep the description, the commit titles and the diff consistent: a pull request described as test only must not modify production code, and every temporary change (debug code, mutation used to validate a test) must be removed before pushing

Pull Request Description

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.

Issue Linking and Assignees

Linking issues

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

Assigning the pull request

Always assign the pull request to yourself. This simplifies tracking and allows reviewers to quickly identify who is responsible for which work.

Copilot Review

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.

Code Review Conduct

Using Conventional Comments (for example **issue (blocking):** or **suggestion (non-blocking):**) is welcome for both reviewers and authors, it makes the intent of each comment explicit.

For reviewers

  • 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

For authors

  • Respond to every comment, even if just to acknowledge it
  • Reply in the thread of the comment, not with a separate top level comment on the pull request
  • Keep replies short: one or two sentences stating what changed, with the commit if relevant
  • Do not post status updates or corrections of previous comments: edit the original comment instead
  • Do not post a comment describing what each new commit includes: the commits and their messages already show it
  • Keep the scope of the pull request stable during the review: new work goes to a follow-up pull request
  • Push fixes as new commits during the review, and squash only when the reviewers ask for it
  • Do not take feedback personally — reviews are about the code, not the person

Resolving Conversations

  • 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

Reviewer Responsibilities

Thoroughness

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.

Rebasing awareness

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.

Review Priority

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.

Review Approvals

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.

External Contributors

External contributors follow the External Contributor Process and all the rules of this document, with the following additions:

  • One issue, one contributor: work only on an issue that a Core team member has assigned to you
  • One issue, one pull request: keep the pull request scoped to the issue, and discuss any scope change on the issue first
  • Follow the conventions of the repository: pull request template, commit message format with the crate scope (for example test(stm): ...), crate version bump and CHANGELOG entry when a crate is modified
  • Be concise: verbose descriptions, replies or code comments slow down the review and are a blocker for merging
  • Respect the reviewers' time: a pull request that repeatedly ignores these rules may be closed without review

AI-Assisted Contributions

Using AI tools to write code, tests or text is allowed, under the following rules:

  • No autonomous agents: an agent must not open pull requests, push commits or post comments without a human reviewing each action beforehand
  • Edit the output: AI generated text must be trimmed to what a reviewer needs, the conciseness rules of this document apply to it as to any other text

A pull request that appears to be driven by an undisclosed autonomous agent may be closed without review.

Asking for Help

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.