Skip to main content

Overview

Pull requests (PRs) are the primary way to contribute code to Aya. A well-crafted PR makes review faster and increases the likelihood of acceptance.
All PRs must pass make ok locally before submission. CI will automatically run the same checks.

Before Creating a PR

1

Ensure Tests Pass

Run all quality checks locally:
make ok
This must pass without errors. Fix any issues before proceeding.
2

Verify Commits

Ensure commits follow Conventional Commits:
git log --oneline
Each commit should have a clear, descriptive message explaining “why” the change was made.
3

Rebase on Main

Sync your branch with the latest main:
git fetch upstream
git rebase upstream/main
Resolve any conflicts and test again.
4

Push to Your Fork

git push origin feat/your-feature-name
Use --force-with-lease if you rebased:
git push --force-with-lease origin feat/your-feature-name

Creating a Pull Request

PR Title Format

Use Conventional Commits format for PR titles:
<type>(<scope>): <description>
Examples:
feat(profiles): add bio field to user profiles
fix(auth): handle null user in session check
refactor(api): extract pure functions for testing
docs(contributing): add pull request guidelines
perf(queries): optimize story_tx locale fallback
See Commit Types for full list.

PR Description Template

Use this structure for PR descriptions:
## Summary

Brief description of what this PR does and why.

## Changes

- List key changes
- One bullet per significant change
- Reference files or modules affected

## Testing

- [ ] All tests pass (`make ok`)
- [ ] New tests added for new functionality
- [ ] Manually tested in browser/CLI

## Screenshots (if applicable)

<!-- Add screenshots for UI changes -->

## Related Issues

Closes #123
References #456

## Checklist

- [ ] Code follows [code style guidelines](/contributing/code-style)
- [ ] Commits follow [Conventional Commits](https://www.conventionalcommits.org/)
- [ ] Documentation updated (if needed)
- [ ] No breaking changes (or documented if unavoidable)

Example PR

Title: feat(profiles): add 3-tier locale fallback for translations Description:
## Summary

Implements a 3-tier locale fallback system for `profile_tx` queries to ensure profiles always display content, even when the exact requested locale isn't available.

**Fallback order:**
1. Requested locale (e.g., `ja`)
2. Entity's default locale (e.g., `en`)
3. Any available translation

## Changes

- Updated `GetProfileBySlug` SQL query to use 3-tier fallback subquery
- Refactored `profile_tx` joins to replace 2-tier `OR` logic
- Added `strings.TrimRight` for `CHAR(12)` locale codes
- Added tests for all fallback scenarios

## Testing

- [x] All tests pass (`make ok`)
- [x] New tests added: `TestGetProfileBySlug_LocaleFallback`
- [x] Manually tested with profiles in multiple locales
- [x] Verified fallback behavior with missing translations

## Related Issues

Closes #123
References #45 (original locale fallback discussion)

## Checklist

- [x] Code follows code style guidelines
- [x] Commits follow Conventional Commits
- [x] Documentation updated (CLAUDE.md locale section)
- [x] No breaking changes

PR Size Guidelines

Keep PRs focused and reasonably sized: Good PR size:
  • Single feature or bug fix
  • ~100-500 lines changed
  • 1-10 files modified
  • Reviewable in 15-30 minutes
Too large:
  • Multiple unrelated changes
  • 1000+ lines changed
  • Affects many subsystems
  • Takes hours to review
Large refactoring? Break it into multiple PRs:
  1. PR 1: Add new interfaces/abstractions (no behavior change)
  2. PR 2: Migrate module A to new abstractions
  3. PR 3: Migrate module B to new abstractions
  4. PR 4: Remove old code

Code Review Process

What Reviewers Look For

  • Follows code style guidelines
  • Uses explicit null/undefined checks (no truthy/falsy)
  • Proper error handling and logging
  • Self-documenting code with meaningful names
  • Comments explain “why”, not “what”
  • Frontend: Uses backend object, CSS Modules, single props pattern
  • Backend: Follows hexagonal architecture, business logic has no external deps
  • No circular dependencies
  • Appropriate separation of concerns
  • New functionality has tests
  • Tests are clear and focused
  • Edge cases covered
  • All tests pass (make ok)
  • Public APIs documented
  • Complex logic explained
  • README or docs updated if needed
  • Breaking changes clearly noted
  • Commits follow Conventional Commits
  • Commit messages are descriptive
  • No merge commits (rebased)
  • No debug code, console.logs, TODOs

Review Workflow

1

Automated Checks Run

CI automatically runs:
  • Frontend linting and tests (deno lint, deno task test:ci)
  • Backend linting and tests (make lint, make test)
  • Build verification
PR cannot be merged until CI passes.
2

Maintainer Review

A maintainer will review your PR, typically within 2-3 days.They may:
  • Approve and merge
  • Request changes
  • Ask questions for clarification
  • Suggest improvements
3

Address Feedback

If changes are requested:
  1. Make the requested changes
  2. Commit with descriptive message:
    git add .
    git commit -m "refactor: address PR feedback on error handling"
    
  3. Push to your branch:
    git push origin feat/your-feature
    
  4. Respond to review comments explaining changes
4

Approval and Merge

Once approved, a maintainer will merge your PR.Merge strategy: Squash and merge (commits are squashed into one)

Responding to Review Comments

Best Practices

Do:
  • Be open to feedback and suggestions
  • Ask for clarification if feedback is unclear
  • Explain your reasoning for design decisions
  • Thank reviewers for their time
  • Mark conversations as resolved after addressing them
Don’t:
  • Take feedback personally
  • Argue endlessly about style preferences
  • Ignore feedback or leave comments unresolved
  • Make unrelated changes during review

Example Responses

Good responses:
Thanks for catching that! I’ve updated the null check to be explicit.
Good point about the error handling. I’ve wrapped the error with context and added a test case.
I see your concern. I chose this approach because [reasoning]. What do you think about [alternative]?
Responses to avoid:
It works fine for me. 🤷
That’s just your opinion.
I don’t have time to change that.

Common PR Rejection Reasons

Your PR may be rejected if:
  • make ok doesn’t pass
  • Adds code without tests
  • Uses truthy/falsy checks instead of explicit comparisons
  • Violates hexagonal architecture (business logic with external deps)
  • Contains merge commits instead of rebasing
  • Includes unrelated changes
  • Has vague commit messages
  • Lacks description or context

After Your PR is Merged

1

Sync Your Fork

git checkout main
git fetch upstream
git rebase upstream/main
git push origin main
2

Delete Feature Branch

git branch -d feat/your-feature  # Local
git push origin --delete feat/your-feature  # Remote
3

Celebrate!

Your contribution is now part of Aya! 🎉You’ll be listed in:
  • Project contributors
  • Release notes (for significant contributions)

Special Cases

Use draft PRs for work-in-progress or to get early feedback:
  1. Create PR and mark as “Draft”
  2. Add “WIP” or “[Draft]” to title
  3. Explain what’s incomplete in description
  4. Mark as “Ready for review” when complete
Draft PRs:
  • Won’t be merged
  • CI still runs
  • Good for discussing approach before finalizing
If your PR introduces breaking changes:
  1. Use BREAKING CHANGE: in commit body:
    feat(api)!: change profile endpoint response format
    
    BREAKING CHANGE: Profile endpoint now returns `locale_code` instead of `locale`.
    Clients must update their response parsing.
    
  2. Document migration path in PR description
  3. Update documentation
  4. Add to changelog under “Breaking Changes” section
Breaking PRs require maintainer discussion before merging.
For security vulnerabilities:
  1. Do NOT create a public PR or issue
  2. Email [email protected] privately
  3. Include:
    • Description of vulnerability
    • Steps to reproduce
    • Suggested fix (if you have one)
Maintainers will coordinate a private patch and disclosure timeline.
For documentation changes:
  1. Use docs: prefix in title
  2. No need for extensive testing (typo fixes, clarifications)
  3. Include screenshots for significant formatting changes
  4. Verify links work
Example:
docs(contributing): fix broken link in testing guide

PR Checklist

Before submitting, verify:
  • make ok passes without errors
  • Commits follow Conventional Commits format
  • PR title follows type(scope): description format
  • PR description explains what and why
  • Tests added for new functionality
  • No console.log, debug code, or TODOs
  • Documentation updated (if needed)
  • Branch is rebased on latest main
  • Code follows style guidelines
  • No merge commits (use rebase)
  • Related issues referenced (e.g., “Closes #123”)

Getting Help

If you need help with your PR:
  1. Check Documentation: Review development workflow and code style
  2. Ask in PR Comments: Tag maintainers with questions
  3. GitHub Discussions: For broader questions about approach
  4. Look at Recent PRs: See how others structured similar changes
First PR? Don’t worry about perfection. Maintainers will guide you through the process. The goal is learning and improving, not getting everything right the first time.

Further Reading

Build docs developers (and LLMs) love