Pull Request Guidelines
What to include in every pull request. Checklist for authors and guidelines for reviewers.
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.
Reference: 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.
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
- Start the dev server with
npm run dev - Navigate to
/login - Enter
test@example.com/password123 - Verify redirect to
/dashboard - 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
Reference: 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.
Reference: 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.
Edit this page