Pull Request Guidelines

PR Template Overview

Every pull request at Orcta uses a standard template to ensure consistency, clarity, and thoroughness. The template guides authors through documenting their changes and helps reviewers understand the context quickly.

Standard Template Structure

Summary

Brief description of what this PR does in 2-3 sentences. Focus on the what, not the how.

Motivation

Why are we making this change? Link to the issue or ticket, or explain the user need if no issue exists.

Changes Made

List the key changes in this PR. Helps reviewers understand scope at a glance.

Testing Instructions

Step-by-step guide for reviewers to verify the changes work as expected.

Screenshots or Evidence

For UI changes: before and after screenshots. For backend: test output or API responses.

Pre-Submission Checklist

Items to verify before requesting review. Ensures quality standards are met.

Technical Decisions

Document trade-offs, alternative approaches considered, or technical debt introduced.

Related Work

Links to dependent PRs, related issues, or follow-up work.

Reviewer Notes

Specific areas where you want focused feedback or context that helps reviewers.

Engineering Playbook, Section 5.2

Author Checklist

Complete all items before requesting review. These checks ensure your PR meets quality standards and is ready for team review.

Tests pass locally — Run the full test suite before pushing
Linter passes — No ESLint, Flake8, or golangci-lint errors
Tests added or updated — New functionality includes corresponding tests
Naming conventions followed — Code follows language-specific style guide
Documentation updated — README, inline comments, or Notion docs reflect changes
No secrets committed — Verify with git diff before pushing
Code refactored — Left the codebase better than you found it

Pre-commit Hooks

Many of these checks run automatically via pre-commit hooks. If hooks fail, fix the issues before committing. See the setup guide for installation instructions.

Writing Effective PR Descriptions

Good Summary

"Add user authentication with email and password. Implements login form, session management, and protected routes. Users can now sign in and access their profile pages."

Bad Summary

"Added some auth stuff and fixed a few things. Should work now."

What makes a good summary?

Be specific about what changed. Include the scope and impact. Write for someone who wasn't involved in the work. Avoid vague terms like "fixed stuff" or "updated things."

Good Testing Instructions

1. Start the dev server with npm run dev
2. Navigate to /login
3. Enter test@example.com / password123
4. Verify redirect to /dashboard
5. Check session persists on page refresh

Bad Testing Instructions

"Just test the login feature. Should be obvious."

What makes good testing instructions?

Step-by-step clarity. Include specific values to test with. Explain what success looks like. Don't assume reviewers know your feature intimately.

Guidelines for Reviewers

Code review is a collaborative process to improve code quality and share knowledge. Reviews should be thorough but kind, constructive but efficient.

What to Review

Correctness of the implementation
Code readability and clarity
Test coverage and quality
Edge cases and error handling
Performance implications
Security considerations
Adherence to conventions

How to Review

Respond within 48 hours
Start with what works well
Ask questions before demanding changes
Be specific in your feedback
Suggest alternatives, don't just criticize
Focus on substance over style
Assume positive intent

Engineering Playbook, Section 5.3

Feedback Best Practices

Effective Feedback

"This function could be more readable. Consider extracting the validation logic into a separate helper function. That would make it easier to test and reuse."

Ineffective Feedback

"This is messy. Refactor it."

What makes feedback effective?

Point to specific lines or sections. Explain why something matters. Suggest concrete alternatives. Frame as collaboration, not criticism.

Constructive Question

"I notice we're not handling the case where the API returns a 429. Is rate limiting handled elsewhere, or should we add retry logic here?"

Unconstructive Demand

"You forgot error handling. Add it."

When to ask vs tell?

Ask questions when you're uncertain or want to understand the author's reasoning. Make direct suggestions for clear improvements. Frame as collaboration whenever possible.

Common Review Patterns

Nitpick Pattern

For minor style or preference issues that don't affect functionality, prefix with "Nit:" to indicate it's not blocking. Example: "Nit: This variable name could be more descriptive, but not critical."

Blocking vs Non-Blocking

Be clear about what must be fixed before merge versus what could be done in a follow-up. Use labels like "Blocking:" for critical issues and "Follow-up:" for nice-to-haves.

Learning Opportunity

Use reviews to teach and learn. Share relevant documentation, patterns, or examples. Explain the reasoning behind suggestions so authors understand the principles, not just the fixes.

Response Timeline

Timely reviews keep development moving and prevent context switching.

For Reviewers

First response within 48 hours
Subsequent reviews within 24 hours
If you can't review, say so early
Unblock authors by reviewing incrementally
Prioritize reviews during maker hours

For Authors

Address feedback within 24 hours
Respond to all comments, even simple "Done"
Explain if you disagree with feedback
Mark conversations as resolved when fixed
Request re-review when ready

When Reviews Stall

If feedback sits unaddressed for 3+ days, or a PR waits for review beyond 48 hours, escalate to a tech lead. Sometimes a quick sync call can resolve blockers faster than async comments.

Approval and Merge

Approval Requirements

At least one approval required from a team member. For significant architectural changes, get approval from a tech lead. All CI checks must pass before merging.

Merge Strategy

We use squash merging to keep git history clean. All commits from your feature branch combine into one. Write a clear merge commit message that summarizes the entire feature.

After Merge

Monitor your changes in production for at least one hour. Delete your feature branch. Close related issues. Update documentation if needed.

Engineering Playbook, Section 4.3

Philosophy

Code review is about improving code, not judging people. We assume positive intent, engage collaboratively, and recognize that everyone is learning. Great reviews make the code better and help everyone grow as engineers.