Skip to content

CONTRIBUTING.md

Latest commit

 

History

History
155 lines (121 loc) · 7.88 KB

CONTRIBUTING.md

File metadata and controls

155 lines (121 loc) · 7.88 KB

Contributing Guidelines | The Style of Work

This document outlines the rigorous engineering standards required for all contributions to the Software Engineering Collective (bhos-sec). We treat all internal repositories as production-grade environments. Excellence is not an act, but a habit.

1. The Philosophy of Contribution

Before writing a single line of code, it is critical to understand why we enforce structural rigor.

Why Reviews are Essential

Code reviews are not a rubber-stamping exercise; they are our primary defense against technical debt and our most effective mechanism for knowledge transfer.

  • Quality Assurance: A second pair of eyes catches edge cases, architectural deviations, and algorithmic inefficiencies that the original author might miss.
  • Mentorship: Reviews are technical mentorship sessions. They provide actionable feedback and elevate the entire team's engineering baseline.
  • Shared Ownership: No single person should be the sole point of failure for any module. Reviews distribute system understanding across the Collective.

Why Tests Must Be Written

We do not believe in code that works "on my machine." If it is not reliably tested, it is fundamentally broken.

  • Confidence in Refactoring: Comprehensive tests allow us to evolve the architecture aggressively without fear of regressions.
  • Living Documentation: Well-written tests explain how a module is intended to be used better than any static wiki page.
  • Automated Contracts: Tests enforce the invariants of our systems. When our CI/CD pipeline runs, tests act as the non-negotiable contract that must be fulfilled before a merge can occur.

PR Size Limits: The Art of the Small PR

Giant, monolithic Pull Requests are an anti-pattern. They are impossible to review effectively and serve as a magnet for hidden bugs.

  • The Ideal Size: A PR should solve one specific problem. It should take a reviewer no more than 15-20 minutes to read and comprehend.
  • The Hard Limit: Any PR exceeding 400 lines of code (LOC) (excluding auto-generated code, lock files, or mass renames) is considered too large and will be rejected.
  • If a feature requires more than 400 lines, it must be logically broken down into smaller, independently mergable PRs (e.g., Database schema in PR #1, Core logic in PR #2, API endpoints in PR #3).

2. The Git Workflow

Chaos in version control is unacceptable. We enforce a strictly standardized Git workflow to ensure a clean, auditable history.

The No-Main-Push Policy

Direct commits to the main or master branches are strictly prohibited. All changes must be integrated via PRs. The main branch must always remain stable and deployable.

Branching Strategy

Branches must be scoped to specific tasks and predictably named:

  • Features: feat/feature-name (e.g., feat/auth-middleware)
  • Bug Fixes: fix/issue-description (e.g., fix/memory-leak-db)
  • Chore/Maintenance: chore/task-name (e.g., chore/dependency-updates)
  • Documentation: docs/document-name (e.g., docs/api-spec)

Conventional Commits

All commit messages must adhere to the Conventional Commits specification. This semantic approach drives our automated changelogs.

  • feat: A new feature.
  • fix: A bug fix.
  • chore: Maintenance tasks, dependency bumps.
  • docs: Documentation updates.
  • style: Code style fixes (formatting, missing semicolons, etc.).
  • refactor: Code changes that neither fix a bug nor add a feature, but improve internal structure.

3. Git Command Guidelines

For those transitioning from academic projects to professional workflows, here is the expected sequence of operations.

Starting Work

Always ensure you are branching off the latest state of the main branch to prevent painful merge conflicts later.

# Switch to the main branch
git checkout main

# Pull the latest changes from the remote repository
git pull origin main

# Create and switch to your new strictly-named branch
git checkout -b feat/your-new-feature

Saving Progress

Commit often, but ensure commits represent logical chunks of work. Messages must follow the Conventional Commits format.

# Stage your specific changes (avoid `git add .` unless necessary)
git add src/your_modified_file.py

# Commit with a descriptive conventional message
git commit -m "feat: add user authentication middleware"

Syncing with Main (Handling Conflicts Early)

If main has progressed while you were working on your branch, you must pull those changes into your branch to resolve conflicts locally before opening a PR.

# While on your feature branch, pull the latest main
git pull origin main

# If conflicts occur, resolve them carefully in your editor, then:
git add .
git commit -m "chore: resolve merge conflicts with main"

Pushing and PR Creation

# Push your branch to the remote repository
git push -u origin feat/your-new-feature

After pushing, navigate to GitHub and open a Pull Request. Ensure you fill out the required PR template entirely.

4. Git Troubleshooting & Best Practices

In a high-velocity engineering environment, you will encounter complex Git scenarios. Here is how we handle them systematically.

Stacked Branches (Branch Dependencies)

Sometimes you need to build feature B before feature A is merged into main. Do not merge A locally into main and then branch off. Instead, create a "stacked" branch.

  1. Create branch A from main and start working: 
    git checkout -b feat/A main
git commit -m "feat: initial work on A"
  2. When you need to start feature B which depends on A, branch directly off A, not main: 
    # From branch feat/A
git checkout -b feat/B
  3. The PR Rule: When opening a PR for feat/B, change the "base branch" in GitHub from main to feat/A. Do not target main yet, as it will include all the commits from A and make the PR impossible to review. Once A is merged into main, GitHub will automatically retarget B to main.

Resolving Merge Conflicts Like an Engineer

Merge conflicts are not errors; they are simply Git asking for human intervention when two people modify the same lines of code. Do not panic.

  1. Pull the latest target branch: If you are merging feat/A into main, first get the latest main. 
    git checkout main
git pull origin main
  2. Merge main into your feature branch: 
    git checkout feat/A
git merge main
  3. Resolve the Conflicts: Git will explicitly mark the conflicting areas in your files with <<<<<<<, =======, and >>>>>>>.
    • Open your IDE (VSCode, JetBrains) and use their conflict resolution tools.
    • Look at the incoming change (main) and your current change (feat/A).
    • Decide which code to keep, or manually write a combination of both.
    • Crucial: Delete the Git conflict markers (<<<<<<<, =======, >>>>>>>) before saving.
  4. Complete the Merge: 
    git add <resolved-files>
git commit -m "chore: resolve merge conflicts with main"

(Note: Advanced users may prefer git rebase main instead of git merge main for a cleaner history, but standard merging is the baseline requirement.)

4. Automated Quality Gates (CI/CD)

Human review is fallible; our CI/CD pipelines are not. A 'Green' build is an absolute prerequisite for any merge.

  1. Linting & Formatting: Code must conform to our stylistic tenets without exception. Python code is strictly enforced via Black, and JavaScript/TypeScript via ESLint.
  2. Unit Testing: Regressions or failing tests will immediately block the build.
  3. Security Scanning: We employ CodeQL to automatically detect vulnerabilities, hardcoded secrets, and anti-patterns before human intervention.

By contributing to the Collective, you agree to maintain the high standards of this engineering culture.