Thank you for your interest in contributing to GitHub Activity DB! This document outlines our quality standards, coding conventions, and the process for submitting contributions.
- Code of Conduct
- Getting Started
- Quality Standards
- Type Safety Policy
- Code Style Guidelines
- Testing Requirements
- Pull Request Process
- Be respectful and inclusive
- Provide constructive feedback
- Focus on the code, not the person
- Help others learn and grow
- Fork the repository and clone your fork
- Install dependencies:
uv sync --all-extras - Set up pre-commit hooks:
uv run pre-commit install - Run tests to verify your setup:
uv run pytest
See Development Guide for detailed setup instructions.
This project maintains strict code quality standards with automated enforcement. All contributions must pass these checks before merging.
| Metric | Target | Enforcement |
|---|---|---|
type: ignore comments in src/ |
≤5 | Pre-commit audit |
noqa comments in src/ |
≤3 | Pre-commit audit |
Any type aliases |
0 | Pre-commit blocks |
| Mypy errors (src/ + tests/) | 0 | Pre-commit blocks |
| Test coverage (critical paths) | 100% | Manual review |
All commits must pass the following automated checks:
# Run all checks before committing
uv run pre-commit run --all-files| Hook | Purpose | Action on Failure |
|---|---|---|
mypy |
Type checking | Blocks commit |
ruff |
Linting | Auto-fixes or blocks |
ruff-format |
Formatting | Auto-fixes |
audit-type-ignores |
Count suppressions | Reports (target ≤5) |
audit-noqa |
Count suppressions | Reports (target ≤3) |
prevent-any-aliases |
Block lazy types | Blocks commit |
We believe in:
- Fix the root cause, not the symptom - Don't suppress type errors; fix the underlying type issue
- Explicit over implicit - Type annotations should be clear and specific
- Maintainability over convenience - A few extra lines of typed code is better than
Any - Automation over documentation - Enforce standards through tooling, not just guidelines
- All public functions must have type annotations
- All class attributes must have type annotations
- Use
from __future__ import annotationsfor forward references
| Use Case | Allowed | Alternative |
|---|---|---|
| JSON/dict serialization | dict[str, Any] |
N/A (legitimate use) |
| Pydantic validators | v: Any |
N/A (Pydantic pattern) |
| ORM factories | from_orm(obj: Any) |
N/A (accepts any ORM) |
| Log context binding | **context: Any |
N/A (dynamic logging) |
| Lazy initialization | Forbidden | Use T | None with TYPE_CHECKING |
| Avoiding generics | Forbidden | Use TYPE_CHECKING block |
| Third-party without types | Document | Add stub or TYPE_CHECKING |
- Must include error code:
# type: ignore[arg-type] - Must have justification if not obvious from context
- Prefer fixing the underlying type issue
- Tracked: Pre-commit reports count (target: ≤5)
Good:
# Pydantic computed_field returns property, not method
@computed_field # type: ignore[prop-decorator]
@property
def usage_percent(self) -> float:
return ...Bad:
# Avoid: suppressing without understanding
result = something() # type: ignore- Centralize in shared utility functions when possible
- Document why the rule doesn't apply
- Tracked: Pre-commit reports count (target: ≤3)
- DRY (Don't Repeat Yourself) - Extract common patterns into shared utilities
- Single Responsibility - Each function/class should do one thing well
- Explicit over Implicit - Be clear about types and intentions
- Fail Fast - Validate inputs early and raise clear errors
# Modules: lowercase_with_underscores
github_client.py
rate_limit_monitor.py
# Classes: PascalCase
class PullRequestRepository:
class GitHubClient:
# Functions/methods: lowercase_with_underscores
def get_pull_request():
async def sync_repository():
# Constants: UPPERCASE_WITH_UNDERSCORES
MAX_RETRIES = 3
DEFAULT_TIMEOUT = 30Imports are automatically organized by ruff. The order is:
- Standard library
- Third-party packages
- Local application imports
from __future__ import annotations
import asyncio
from datetime import UTC, datetime
from typing import TYPE_CHECKING
import typer
from pydantic import BaseModel
from github_activity_db.db.models import PullRequest
from github_activity_db.schemas import PRCreate
if TYPE_CHECKING:
from githubkit import GitHubUse async/await consistently:
# Good: async context manager
async with GitHubClient() as client:
result = await client.get_pull_request(owner, repo, number)
# Good: async iteration
async for pr in client.iter_pull_requests(owner, repo):
await process_pr(pr)All CLI commands should use run_async_command() for unified error handling:
from github_activity_db.cli.common import console, run_async_command
@app.command()
def my_command() -> None:
"""Command description."""
async def _impl() -> dict[str, Any]:
async with GitHubClient() as client:
return await client.some_method()
result = run_async_command(_impl())
console.print(f"Result: {result}")| Category | Requirement |
|---|---|
| Critical paths | 100% coverage |
| New features | Tests required |
| Bug fixes | Regression test required |
| Edge cases | High coverage |
1. Use model_validate() for Pydantic models:
# Good
pr = GitHubPullRequest.model_validate(GITHUB_PR_RESPONSE)
# Bad - loses type information
pr = GitHubPullRequest(**GITHUB_PR_RESPONSE)2. Assert on optional values before access:
result = await repo.get_by_number(...)
assert result is not None # Narrows type
assert result.state == PRState.MERGED3. Use factory functions with explicit parameters:
pr = make_pull_request(
db_session,
repo,
number=1234,
state=PRState.OPEN,
title="Test PR",
)# Run all tests
uv run pytest
# Run with coverage
uv run pytest --cov=src/github_activity_db --cov-report=term-missing
# Run specific test file
uv run pytest tests/test_db_models.py -vSee Testing Guide for comprehensive testing documentation.
-
Run all checks locally:
uv run pre-commit run --all-files uv run pytest
-
Ensure mypy passes:
uv run mypy src/ tests/
-
Check quality metrics:
grep -r "type: ignore" src/ | wc -l # Should be ≤5 grep -r "noqa:" src/ | wc -l # Should be ≤3
- All pre-commit hooks pass
- All tests pass
- New code has test coverage
- Type annotations are complete
- No new
type: ignorewithout justification - No new
noqawithout justification - Documentation updated if needed
## Summary
Brief description of changes
## Changes
- Change 1
- Change 2
## Testing
How was this tested?
## Checklist
- [ ] Pre-commit hooks pass
- [ ] Tests pass
- [ ] Documentation updated- Automated checks must pass
- At least one approval required
- All comments must be resolved
- Squash and merge preferred
- Check the Development Guide
- Check the Testing Guide
- Open an issue for questions or suggestions
Thank you for contributing!