-
Notifications
You must be signed in to change notification settings - Fork 39
docs: add comprehensive contributing guide #847
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
mergify
merged 1 commit into
python-wheel-build:main
from
LalatenduMohanty:coding_best_practices_guide
Dec 8, 2025
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,138 +1,216 @@ | ||
| # Contributor Quickstart Guide | ||
| # AI Agent Guide for Fromager | ||
|
|
||
| ## Commit Message Guidelines | ||
| > **Note**: This file is also available as `CLAUDE.md` (symlink) for Claude Code CLI users. | ||
| > | ||
| > **IMPORTANT**: Before making any code changes, you MUST read [CONTRIBUTING.md](CONTRIBUTING.md) for comprehensive coding standards and design patterns. This file provides essential quick reference only. | ||
|
|
||
| ### Objectives | ||
| ## When to Read CONTRIBUTING.md | ||
|
|
||
| - Help the user craft commit messages that follow best practices | ||
| - Use [Conventional Commit](https://www.conventionalcommits.org/en/v1.0.0/) format unless otherwise specified | ||
| - Clarify unclear or incomplete input with targeted questions | ||
| - Ensure messages are concise, informative, and use imperative mood | ||
| **Always read CONTRIBUTING.md before:** | ||
|
|
||
| ### Style Guidelines | ||
| - Writing new functions (for type annotation standards) | ||
| - Adding imports (for import organization rules) | ||
| - Creating tests (for testing patterns) | ||
| - Making commits (for commit message format) | ||
| - Adding error handling or logging | ||
|
|
||
| - Use the format: `<type>(<scope>): <short summary>` for the subject line | ||
| - Keep the subject line ≤ 72 characters | ||
| - Use a blank line before the body | ||
| - The body explains what and why (not how) | ||
| - Use a footer for metadata (e.g., `Closes: #123`, `BREAKING CHANGE:`) | ||
| ## Essential Rules (MUST FOLLOW) | ||
|
|
||
| ### Commit Types | ||
| ### Do | ||
|
|
||
| - **feat**: a new feature | ||
| - **fix**: a bug fix | ||
| - **docs**: documentation only changes | ||
| - **style**: formatting, missing semi colons, etc | ||
| - **refactor**: code change that neither fixes a bug nor adds a feature | ||
| - **perf**: performance improvements | ||
| - **test**: adding missing tests | ||
| - **chore**: changes to the build process or auxiliary tools | ||
| - **Type annotations REQUIRED** on ALL functions including tests. Use syntax compatible with Python 3.11+ | ||
| - Use `X | None` not `Optional[X]` | ||
| - Add docstrings on all public functions and classes | ||
| - Use file-scoped commands for fast feedback (see below) | ||
| - Follow existing patterns - search codebase for similar code | ||
| - Chain exceptions: `raise ValueError(...) from err` | ||
| - Use `req_ctxvar_context()` for per-requirement logging | ||
| - Run `hatch run lint:fix` to format code (handles line length, whitespace, etc.) | ||
|
|
||
| ### Examples | ||
| ### Don't | ||
|
|
||
| #### Good commit messages | ||
| - Don't use `Optional[X]` syntax (use `X | None`) | ||
| - Don't omit type annotations or return types | ||
| - Don't run full test suite for small changes (use file-scoped) | ||
| - Don't create temporary helper scripts or workarounds | ||
| - Don't commit without running quality checks | ||
| - Don't make large speculative changes without asking | ||
| - Don't update git config or force push to main | ||
| - Don't use bare `except:` - always specify exception types | ||
|
|
||
| ```text | ||
| feat(api): add user authentication endpoint | ||
| ## Commands (IMPORTANT: Use File-Scoped First) | ||
|
|
||
| Add JWT-based authentication system for secure API access. | ||
| Includes token generation, validation, and refresh functionality. | ||
| ### File-Scoped Commands (PREFER THESE) | ||
|
|
||
| Closes: #123 | ||
| ``` | ||
| ```bash | ||
| # Type check single file | ||
| hatch run mypy:check <filepath> | ||
|
|
||
| ```text | ||
| fix(parser): handle empty input gracefully | ||
| # Format single file | ||
| hatch run lint:fix <filepath> | ||
|
|
||
| # Test specific file | ||
| hatch run test:test tests/test_<module>.py | ||
|
|
||
| Previously, empty input would cause a null pointer exception. | ||
| Now returns an appropriate error message. | ||
| # Test specific function | ||
| hatch run test:test tests/test_<module>.py::test_function_name | ||
|
|
||
| # Debug test with verbose output | ||
| hatch run test:test <filepath> --log-level DEBUG | ||
| ``` | ||
|
|
||
| ```text | ||
| docs: update installation instructions | ||
| ### Project-Wide Commands (ASK BEFORE RUNNING) | ||
|
|
||
| Add missing dependency requirements and clarify setup steps | ||
| for new contributors. | ||
| ```bash | ||
| hatch run lint:fix # Format all code | ||
| hatch run test:test # Full test suite (slow!) | ||
| hatch run mypy:check # Type check everything | ||
| hatch run lint:check # Final lint check | ||
| ``` | ||
|
|
||
| #### Poor commit messages to avoid | ||
| ## Safety and Permissions | ||
|
|
||
| - `fix bug` (too vague) | ||
| - `updated files` (not descriptive) | ||
| - `WIP` (not informative) | ||
| - `fixed the thing that was broken` (not professional) | ||
| ### Allowed Without Asking | ||
|
|
||
| ### Best Practices | ||
| - Read files, search codebase | ||
| - Run file-scoped linting, type checking, tests | ||
| - Edit existing files following established patterns | ||
| - Create test files | ||
|
|
||
| - Write in imperative mood (e.g., "add feature" not "added feature") | ||
| - Don't end the subject line with a period | ||
| - Use the body to explain the motivation for the change | ||
| - Reference issues and pull requests where relevant | ||
| - Use `BREAKING CHANGE:` in footer for breaking changes | ||
| ### Ask First | ||
|
|
||
| ## Code Quality Guidelines | ||
| - Installing/updating packages in pyproject.toml | ||
| - Git commit or push operations | ||
| - Deleting files or entire modules | ||
| - Running full test suite | ||
| - Creating new modules or major refactors | ||
| - Making breaking changes | ||
|
|
||
| ### Formatting Standards | ||
| ## Project Structure | ||
|
|
||
| - **No trailing whitespace**: Ensure no extra spaces at the end of lines | ||
| - **No whitespace on blank lines**: Empty lines should contain no spaces or tabs | ||
| - **End files with a single newline**: Each file should end with a single newline character (`\n`). This is a widely adopted convention recommended by PEP 8, Python's style guide. Many Unix-style text editors and tools expect files to end with a newline character and may not handle files without one properly. | ||
| - Follow the project's existing code style and indentation patterns | ||
| - Use consistent line endings (LF for this project) | ||
| - `src/fromager/` - Main package code | ||
| - `tests/` - Unit tests (mirror `src/` structure) | ||
| - `e2e/` - End-to-end integration tests | ||
| - `docs/` - Sphinx documentation | ||
|
|
||
| ### Type Annotations | ||
| ### Reference Files for Patterns | ||
|
|
||
| - **Always add type annotations to all functions**: All functions must include type annotations for parameters and return values | ||
| - This applies to regular functions, test functions, class methods, and async functions | ||
| - Existing code will be updated gradually; new code must be fully typed | ||
| - Follow the existing pattern in the codebase for consistency | ||
| - Examples: | ||
| **Before writing code, look at these examples:** | ||
|
|
||
| ```python | ||
| def calculate_total(items: list[int]) -> int: | ||
| """Calculate sum of items.""" | ||
| return sum(items) | ||
| - Type annotations: `src/fromager/context.py` | ||
| - Pydantic models: `src/fromager/packagesettings.py` | ||
| - Logging with context: `src/fromager/resolver.py` | ||
| - Error handling: `src/fromager/commands.py` | ||
| - Testing patterns: `tests/test_context.py` | ||
|
|
||
| def test_my_feature() -> None: | ||
| """Test that my feature works correctly.""" | ||
| assert my_feature() == expected_result | ||
| ``` | ||
| ## Code Patterns | ||
|
|
||
| ### Code Comments | ||
| **Import Guidelines:** | ||
|
|
||
| - **Avoid unnecessary comments**: Write self-documenting code with clear variable names, function names, and structure | ||
| - **Only add comments when absolutely necessary**: Comments should be reserved for must-have cases such as: | ||
| - Explaining complex algorithms or non-obvious logic | ||
| - Documenting "why" decisions were made (not "what" the code does) | ||
| - Warning about edge cases or subtle bugs | ||
| - Explaining workarounds for external library issues | ||
| - **Do not add comments that simply restate what the code does**: The code itself should be clear enough | ||
| - **Prefer docstrings over comments**: Use docstrings for functions, classes, and modules to document their purpose and usage | ||
| - **PEP 8: imports should be at the top**: All import statements must be placed at the top of the file, after module docstrings and before other code | ||
| - **No local imports**: Do not place import statements inside functions, methods, or conditional blocks | ||
|
|
||
| ### Testing After Code Changes | ||
| ### Testing Pattern | ||
|
|
||
| After making code changes, run the following tests within a Python virtual environment to ensure code quality: | ||
| ```python | ||
| def test_behavior(tmp_path: pathlib.Path) -> None: | ||
| """Verify expected behavior.""" | ||
| # Arrange | ||
| config = tmp_path / "config.txt" | ||
| config.write_text("key=value\n") | ||
| # Act | ||
| result = load_config(config) | ||
| # Assert | ||
| assert result["key"] == "value" | ||
| ``` | ||
|
|
||
| #### Run all unit tests | ||
| ## Commit Message Format (REQUIRED) | ||
|
|
||
| ```bash | ||
| hatch run test:test | ||
| Use [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) format: | ||
|
|
||
| ```text | ||
| <type>(<scope>): <short summary> | ||
|
|
||
| <body explaining what + why> | ||
|
|
||
| <footer: Closes: #123> | ||
| ``` | ||
|
|
||
| #### Run lint checks | ||
| ### Types | ||
|
|
||
| ```bash | ||
| hatch run lint:check | ||
| - **feat**: new functionality | ||
| - **fix**: bug fix | ||
| - **docs**: documentation only | ||
| - **test**: tests only | ||
| - **refactor**: behavioral no-op refactor | ||
| - **perf**: performance improvement | ||
| - **chore**: tooling or dependency change | ||
|
|
||
| ### Good Examples | ||
|
|
||
| ```text | ||
| feat(resolver): add exponential backoff for HTTP retries | ||
|
|
||
| Improves resilience when PyPI is under load by adding jittered backoff. | ||
|
|
||
| Closes: #123 | ||
| ``` | ||
|
|
||
| #### Run mypy type checking | ||
| ```text | ||
| fix(constraints): handle missing constraint file gracefully | ||
|
|
||
| Validate file existence and emit helpful message instead of crashing. | ||
| ``` | ||
|
|
||
| ```bash | ||
| hatch run mypy:check | ||
| ### AI Agent Attribution | ||
|
|
||
| When AI agents create or significantly modify code, add attribution using `Co-Authored-By`: | ||
|
|
||
| ```text | ||
| feat(resolver): add exponential backoff for HTTP retries | ||
|
|
||
| Improves resilience when PyPI is under load by adding jittered backoff. | ||
|
|
||
| Co-Authored-By: Claude <claude@anthropic.com> | ||
| Closes: #123 | ||
| ``` | ||
|
|
||
| ### Before Committing | ||
| ### Bad Examples (NEVER DO THIS) | ||
|
|
||
| - `fix bug` (too vague) | ||
| - `updated files` (not descriptive) | ||
| - `WIP` (not informative) | ||
| - `fixed the thing that was broken` (not professional) | ||
|
|
||
| ## Workflow for Complex Tasks | ||
|
|
||
| 1. **Search codebase** for similar patterns first | ||
| 2. **Create a checklist** in a markdown file for tracking | ||
| 3. **Work through items systematically** one at a time | ||
| 4. **Run file-scoped tests** after each change | ||
| 5. **Check off completed items** before moving to next | ||
| 6. **Run full quality checks** only at the end | ||
|
|
||
| ## When Uncertain | ||
|
|
||
| - Ask clarifying questions rather than making assumptions | ||
| - Search the codebase for similar patterns before inventing new ones | ||
| - Propose a specific plan before making large changes | ||
| - Reference [CONTRIBUTING.md](CONTRIBUTING.md) for detailed guidance | ||
| - **DO NOT** make large speculative changes without confirmation | ||
|
|
||
| ## Quality Checklist Before Finishing | ||
|
|
||
| - [ ] Read CONTRIBUTING.md for relevant standards | ||
| - [ ] Type annotations on all functions | ||
| - [ ] Docstrings on public APIs | ||
| - [ ] Tests cover the change | ||
| - [ ] File-scoped tests pass | ||
| - [ ] No trailing whitespace | ||
| - [ ] File ends with single newline | ||
| - [ ] Conventional Commit format used | ||
| - [ ] Full quality checks pass: `hatch run lint:fix && hatch run test:test && hatch run mypy:check && hatch run lint:check` | ||
|
|
||
| --- | ||
|
|
||
| - Review your changes for trailing whitespace: `git diff | grep -E "^\+.*[[:space:]]$"` | ||
| - Run tests to ensure all changes work correctly | ||
| - Check for linting errors if the project uses linters | ||
| **See [CONTRIBUTING.md](CONTRIBUTING.md) for comprehensive standards, detailed examples, and design patterns used in Fromager.** | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.