Skip to content

Latest commit

 

History

History
658 lines (493 loc) · 15.8 KB

File metadata and controls

658 lines (493 loc) · 15.8 KB

Contributing to oxidizePdf

Thank you for your interest in contributing to oxidizePdf! This document provides guidelines and information for contributors.

Getting Started

Prerequisites

  • Rust 1.70+ (stable)
  • Git
  • A GitHub account

Development Setup

  1. Fork and Clone

    git clone https://github.com/your-username/oxidizePdf.git
    cd oxidizePdf
  2. Install Dependencies

    cargo build --workspace
  3. Run Tests

    cargo test --workspace
  4. Running Tests with Real PDFs (Optional)

    # Enable tests with real PDF fixtures
    cargo test --workspace --features real-pdf-tests
    
    # Note: This requires PDF fixtures in tests/fixtures/
    # Without the feature, these tests are ignored

Development Workflow

Before Making Changes

  1. Create a Feature Branch

    git checkout -b feature/your-feature-name
  2. Ensure Clean Starting Point

    cargo fmt --all
    cargo clippy --all -- -D warnings
    cargo test --workspace

Making Changes

  1. Follow Development Guidelines

    • See docs/DEVELOPMENT_GUIDELINES.md for detailed patterns
    • Use cargo fmt --all for consistent formatting
    • Address all clippy warnings: cargo clippy --all -- -D warnings
  2. Write Tests

    • Add unit tests for new functionality
    • Update integration tests if needed
    • Ensure all tests pass: cargo test --workspace
  3. Document Changes

    • Update public API documentation
    • Add examples for new features
    • Update CHANGELOG.md for significant changes

Pre-commit Validation

Automated Pre-commit Hook (Recommended)

The project includes a pre-commit hook that automatically validates your changes:

# Install the hook (one-time setup)
./scripts/install-hooks.sh

The hook will check:

  1. ✅ No backup files (*.backup, *_old.rs)
  2. ✅ Code formatting (cargo fmt)
  3. ✅ Clippy lints (including performance checks from Sprint 2)
  4. ✅ Library build success
  5. ✅ Library tests passing

Manual Validation (if hook disabled):

# 1. Format code
cargo fmt --all

# 2. Check for warnings (treat as errors)
cargo clippy --workspace --lib -- -D warnings \
  -W clippy::redundant_clone \
  -W clippy::inefficient_to_string \
  -W clippy::assertions_on_constants

# 3. Run all tests
cargo test --workspace

# 4. Build entire workspace
cargo build --workspace

Note: The hook runs library checks only for speed (~2-3 min). Run cargo test --workspace before pushing to catch integration test failures.

Submitting Changes

  1. Commit with Descriptive Messages

    git commit -m "feat: add PDF merge functionality
    
    - Implement MergeOptions for customization
    - Add comprehensive tests
    - Update API documentation"
  2. Push to Your Fork

    git push origin feature/your-feature-name
  3. Create Pull Request

    • Use the GitHub web interface
    • Fill out the PR template
    • Link related issues

GitFlow Workflow

This project follows a strict GitFlow workflow. Understanding and following this workflow is mandatory for all contributions.

Branch Structure

main (production-ready code)
│
└── development (integration branch)
    │
    ├── feature/* (new features)
    ├── release/* (release preparation)
    └── hotfix/* (critical production fixes)

GitFlow Rules

1. Feature Development

All new features MUST:

  • Be created from development branch
  • Be merged back to development branch
  • NEVER be merged directly to main
# Create feature branch
git checkout development
git pull origin development
git checkout -b feature/new-feature

# Work on feature
git add .
git commit -m "feat: implement new feature"

# Merge back to development
git checkout development
git merge feature/new-feature
git push origin development

2. Release Process

Releases are created from development and merged to BOTH main and development:

# Create release branch
git checkout development
git checkout -b release/v1.2.0

# Bump version, final fixes
# Update CHANGELOG.md
git commit -m "chore: prepare release v1.2.0"

# Merge to main
git checkout main
git merge --no-ff release/v1.2.0
git tag v1.2.0
git push origin main --tags

# Merge back to development
git checkout development
git merge --no-ff release/v1.2.0
git push origin development

# Delete release branch
git branch -d release/v1.2.0

3. Hotfix Process

Hotfixes are the ONLY branches that can be created from main:

# Create hotfix from main
git checkout main
git checkout -b hotfix/critical-bug

# Fix the bug
git commit -m "fix: resolve critical production issue"

# Merge to main
git checkout main
git merge --no-ff hotfix/critical-bug
git tag v1.1.1
git push origin main --tags

# IMMEDIATELY merge to development
git checkout development
git merge --no-ff hotfix/critical-bug
git push origin development

# Delete hotfix branch
git branch -d hotfix/critical-bug

Critical Rules

  1. NEVER create feature branches from tags or main
  2. NEVER merge features directly to main
  3. ALWAYS ensure development contains everything in main
  4. Hotfixes MUST be merged to both main AND development

Common Mistakes to Avoid

Wrong:

# Creating feature from main or tag
git checkout main
git checkout -b feature/new-feature

# Merging feature directly to main
git checkout main
git merge feature/new-feature

Correct:

# Always from development
git checkout development
git checkout -b feature/new-feature

# Always back to development
git checkout development
git merge feature/new-feature

Branch Naming Conventions

  • Features: feature/descriptive-name
  • Releases: release/vX.Y.Z
  • Hotfixes: hotfix/critical-issue-name

Pull Request Guidelines for GitFlow

  1. Feature PRs: Target development branch
  2. Release PRs: Create two PRs - one to main, one to development
  3. Hotfix PRs: Create two PRs - one to main, one to development

CI/CD Pipeline

Automated Checks

Our CI/CD pipeline runs on every PR and includes:

  • Format Check: cargo fmt --check
  • Clippy Linting: cargo clippy --all -- -D warnings
  • Build Verification: cargo build --workspace
  • Test Suite: cargo test --workspace
  • Code Coverage: Coverage analysis and reporting

Troubleshooting Failed Pipelines

Format Failures

# Fix formatting issues
cargo fmt --all

# Verify locally
cargo fmt --all -- --check

Clippy Failures

Common patterns and fixes are documented in docs/DEVELOPMENT_GUIDELINES.md.

Quick fixes for common issues:

# Modern I/O errors
std::io::Error::other("message")  // Instead of Error::new(ErrorKind::Other, "message")

# Avoid unnecessary clones
map.insert(*id, value)  // Instead of map.insert(id.clone(), value)

# Prefer value iteration
for value in map.values()  // Instead of for (_, value) in &map

# Use is_empty()
!vec.is_empty()  // Instead of vec.len() > 0

Build Failures

  • Check for missing dependencies in Cargo.toml
  • Verify feature flags are correctly configured
  • Ensure cross-platform compatibility

Test Failures

# Run specific test
cargo test test_name -- --nocapture

# Run tests with output
cargo test -- --nocapture

# Run only integration tests
cargo test --test integration_test_name

Code Style and Standards

Rust Guidelines

  • Follow Rust API Guidelines
  • Use rustfmt default configuration
  • Address all clippy warnings
  • Prefer explicit error handling over unwrap()

Anti-Patterns to Avoid

Based on community feedback and code reviews, please avoid these patterns:

❌ Bool + Option Pattern

Bad:

struct ProcessingResult {
    success: bool,
    error: Option<String>,
    data: Option<Data>,
}

Why: Allows impossible states (success: true with error: Some(...)).

Good:

struct ProcessingResult {
    filename: String,
    duration: Duration,
    result: Result<Data, Error>,
}

Rationale: Result<T, E> enforces mutual exclusivity and is idiomatic Rust.

❌ Primitives for Duration

Bad:

struct Metrics {
    duration_ms: u64,  // What if we need microseconds later?
}

Good:

struct Metrics {
    duration: Duration,  // Self-documenting, flexible
}

Rationale: Duration provides type safety, prevents unit confusion, and offers rich API.

⚠️ When .cloned().collect() is OK

.cloned().collect() is NOT automatically wrong. It's correct when:

// ✅ Avoiding borrow conflicts
let keys: Vec<String> = dict.keys().cloned().collect();
for key in keys {
    dict.remove(&key);  // Mutating dict - can't hold reference
}

// ✅ API contract requires owned values
pub fn get_keys(&self) -> Vec<String> {
    self.map.keys().cloned().collect()
}

// ❌ Unnecessary clone
let data: Vec<_> = vec.iter().cloned().collect();  // Just use vec.clone()

Rule of thumb: If you're cloning to avoid borrow checker issues or to satisfy an API contract, it's fine.

Custom Lints

We use dylint to enforce project-specific patterns:

# Run custom lints
./scripts/run_lints.sh

See docs/LINTS.md for details on our 5 production lints.

Documentation

  • Document all public APIs with /// comments
  • Include examples in documentation comments
  • Use cargo doc --open to verify documentation builds
  • Keep examples simple and focused

Testing

  • Write unit tests for all new functionality
  • Add integration tests for complex workflows
  • Use descriptive test names: test_merge_preserves_bookmarks
  • Mock external dependencies appropriately

Real PDF Testing

Tests that require real PDF files are gated behind the real-pdf-tests feature flag:

#[test]
#[cfg_attr(not(feature = "real-pdf-tests"), ignore = "real-pdf-tests feature not enabled")]
fn test_with_real_pdfs() {
    // Test code that requires actual PDF files
}

To run tests with real PDFs:

# Place PDF files in tests/fixtures/
# Run tests with the feature enabled
cargo test --features real-pdf-tests

This approach ensures:

  • CI/CD pipelines run quickly with synthetic PDFs
  • Local development can test against real PDFs when needed
  • No copyrighted material is checked into the repository

Community Guidelines

Code of Conduct

  • Be respectful and inclusive
  • Focus on constructive feedback
  • Help others learn and grow
  • Follow GitHub's community guidelines

Communication

  • Issues: Report bugs and request features
  • Discussions: Ask questions and share ideas
  • PRs: Submit code changes
  • Email: For security issues only

Types of Contributions

🐛 Bug Reports

  • Use the bug report template
  • Include minimal reproduction steps
  • Provide system information
  • Check for existing reports first

✨ Feature Requests

  • Use the feature request template
  • Explain the use case
  • Consider backwards compatibility
  • Discuss design before implementation

📚 Documentation

  • Improve API documentation
  • Add usage examples
  • Fix typos and grammar
  • Translate documentation

🧪 Testing

  • Add missing test coverage
  • Improve test reliability
  • Create property-based tests
  • Add performance benchmarks

Project Structure

oxidizePdf/
├── oxidize-pdf-core/     # Core PDF library
├── oxidize-pdf-cli/      # Command-line interface
├── oxidize-pdf-api/      # REST API server
├── test-suite/           # Integration tests
├── docs/                 # Documentation
├── .github/              # GitHub workflows
└── README.md

CI/CD Pipeline

oxidizePdf uses GitHub Actions for continuous integration and delivery. Understanding the CI pipeline helps you anticipate checks that will run on your pull requests.

CI Workflow Overview

File: .github/workflows/ci.yml

The CI pipeline runs on every push and pull request to ensure code quality:

1. Code Quality Checks

- Formatting (cargo fmt --check)
- Linting (cargo clippy)
- Backup file detection (*.backup, *_old.rs)

Sprint 2 Enhancements: Clippy now includes performance lints:

  • clippy::redundant_clone - Detects unnecessary clones
  • clippy::inefficient_to_string - Catches double-string conversions
  • clippy::assertions_on_constants - Removes useless assertions

2. Build & Test Matrix

Tests run across multiple platforms:

  • Linux (ubuntu-latest)
  • macOS (macos-latest)
  • Windows (windows-latest)

Each platform runs:

cargo build --workspace
cargo test --workspace

3. Coverage Reporting

File: .github/workflows/coverage.yml

  • Runs Tarpaulin for code coverage
  • Reports to Codecov
  • Current Coverage: 54.03% (18,674/34,565 lines)
  • Target: 60% by end of Sprint 3

Coverage runs weekly and on main branch updates.

Release Workflow

File: .github/workflows/release.yml

Triggered by version tags (v*):

  1. Build & Test: Full workspace validation
  2. Publish to crates.io:
    • oxidize-pdf (core library)
    • oxidize-pdf-cli (command-line tool)
    • oxidize-pdf-api (REST API server)
  3. Create GitHub Release: With generated changelog

Example:

git tag v1.7.0
git push origin v1.7.0
# GitHub Actions handles the rest

Local CI Simulation

Run the same checks CI will run:

# Full CI simulation
./scripts/ci-check.sh  # TODO: Create this script

# Or manually:
cargo fmt --all --check
cargo clippy --workspace --all-targets -- -D warnings \
  -W clippy::redundant_clone \
  -W clippy::inefficient_to_string
cargo test --workspace
cargo build --workspace --release

CI Performance Tips

For Contributors:

  • Pre-commit hook catches issues before CI (~2-3 min vs ~10 min CI)
  • Fix clippy warnings early (CI treats warnings as errors)
  • Run cargo test --workspace locally (CI matrix runs 3x platforms)

For Maintainers:

  • Library-only checks are faster for iterative development
  • Full workspace tests before merging to main
  • Coverage runs weekly (expensive, ~15 min)

Troubleshooting CI Failures

"Backup files found":

find . -name "*.backup" -delete
find . -name "*_old.rs" -delete

"Formatting issues":

cargo fmt --all
git add -u && git commit --amend --no-edit

"Clippy warnings":

cargo clippy --workspace --all-targets -- -D warnings
# Fix warnings or add #[allow(...)] with justification

"Tests failing on platform X":

  • Check platform-specific code (Windows paths, macOS security)
  • Use #[cfg(target_os = "...")] for platform conditionals
  • Test locally with Docker or CI artifacts

Future Enhancements (Sprint 3+)

  • Benchmark regression detection (Task 22)
  • Dependency security audits (cargo audit)
  • Documentation generation (rustdoc deployment)
  • Performance profiling (criterion benchmarks in CI)

Getting Help

Resources

Support Channels

  • GitHub Issues: Bug reports and feature requests
  • GitHub Discussions: Questions and community discussion
  • Documentation: In-code examples and API docs

License

By contributing to oxidizePdf, you agree that your contributions will be licensed under the same license as the project (MIT License).

Recognition

Contributors are recognized in:

  • CHANGELOG.md for significant contributions
  • GitHub contributors list
  • Release notes for major features

Thank you for contributing to oxidizePdf! Your efforts help make PDF processing in Rust better for everyone.