From 96ad22b53ee73c4dc41f0b31c99b81a33d5fafcd Mon Sep 17 00:00:00 2001 From: Deniz Ulker <156104354+uelkerd@users.noreply.github.com> Date: Mon, 22 Sep 2025 23:34:44 +0300 Subject: [PATCH 1/4] docs: add comprehensive code quality standards guide MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add CODE_QUALITY_README.md containing complete development standards: - Monster PR prevention strategies and enforcement - Fortress development workflow guidelines - Micro-PR discipline and size limits - Automated quality tooling documentation - Team adoption and success metrics Provides foundation for fortress-compliant development culture. Supports new developer onboarding and process standardization. Extracted from monster PR #171 as part of systematic decomposition. Tracked in issue #174. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- CODE_QUALITY_README.md | 207 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 207 insertions(+) create mode 100644 CODE_QUALITY_README.md diff --git a/CODE_QUALITY_README.md b/CODE_QUALITY_README.md new file mode 100644 index 000000000..57e9950c6 --- /dev/null +++ b/CODE_QUALITY_README.md @@ -0,0 +1,207 @@ +# SAMO-DL Code Quality Standards + +## 🎯 Mission: Prevent Monster PRs Forever + +This document outlines the **strict rules** and **automated enforcement** designed to prevent the creation of massive, unfocused pull requests that slow down development and make code reviews impossible. + +## 🚫 THE PROBLEM WE SOLVE + +**Monster PRs** are pull requests that: +- Change 100+ files +- Add 1000+ lines of code +- Mix multiple concerns (API + tests + docs + infrastructure) +- Take weeks to review +- Cause merge conflicts +- Block progress + +## ✅ THE SOLUTION: Micro-PRs Only + +### **Hard Limits (Automated Enforcement)** +```yaml +# GitHub Actions - PR Size Guard +max_files_changed: 50 # HARD STOP at 50 files +max_lines_changed: 1500 # HARD STOP at 1500 lines +max_commits_per_pr: 5 # HARD STOP at 5 commits +branch_lifetime: 48h # FORCE merge or close after 48h +``` + +### **Single Purpose Rule** +**EVERY PR MUST HAVE EXACTLY ONE SENTENCE DESCRIBING ITS PURPOSE** +- ✅ "Add user authentication system" +- ✅ "Fix memory leak in model loading" +- ❌ "Improve model architecture and fix bugs" *(TWO THINGS!)* +- ❌ "Refactor training pipeline" *(TOO VAGUE!)* + +## 🛠️ AUTOMATED ENFORCEMENT + +### **1. Pre-Commit Hook** +The `.git/hooks/pre-commit` script automatically: +- Validates branch naming (`feat/add-auth`, `fix/memory-leak`) +- Checks commit message format (`feat: add user auth`) +- Prevents commits with mixed concerns +- Runs before every commit + +### **2. PR Scope Checker** +```bash +python scripts/check_pr_scope.py --strict +``` +Validates: +- File count ≤ 50 +- Line changes ≤ 1500 +- Single purpose (no mixing concerns) +- Branch naming compliance + +### **3. CI Pipeline Checks** +GitHub Actions automatically: +- Runs scope validation on PR creation +- Fails builds that exceed limits +- Prevents merging of out-of-scope PRs + +## 📋 DEVELOPMENT WORKFLOW + +### **Before Creating a Branch** +```bash +# Answer these questions: +1. Can I describe this in ONE sentence? +2. Will this affect < 50 files? +3. Can I complete this in < 4 hours? +4. Is this EXACTLY ONE concern? +5. Am I mixing API + tests + docs? + +# If ANY answer is NO: Split into separate PRs +``` + +### **Branch Naming Convention** +``` +feat/short-description # New features +fix/short-description # Bug fixes +chore/short-description # Build/tooling changes +refactor/short-description # Code restructuring +docs/short-description # Documentation +test/short-description # Test additions +``` + +**Examples:** +- ✅ `feat/add-user-auth` +- ✅ `fix/validate-input` +- ✅ `chore/update-deps` +- ✅ `refactor/simplify-logic` +- ❌ `feature/add-auth-and-fix-bugs` *(multiple concerns)* +- ❌ `fix-stuff` *(too vague)* + +### **Commit Message Format** +``` +(): + + +``` +**Examples:** +``` +feat: add JWT token authentication +fix: resolve memory leak in model loading +chore: update Python dependencies +refactor: simplify rate limiter logic +``` + +## 🏗️ CODE QUALITY TOOLS + +### **Automated Tools** +- **Black**: Code formatting (88 char lines) +- **isort**: Import sorting +- **flake8**: Linting and style +- **pylint**: Advanced code analysis +- **mypy**: Type checking +- **bandit**: Security scanning +- **safety**: Dependency vulnerability checks + +### **Pre-commit Hooks** +Run automatically on commit: +```bash +pre-commit install # Install hooks +pre-commit run --all-files # Run on all files +``` + +### **Development Commands** +```bash +make format # Format code +make lint # Run linters +make test # Run tests +make quality-check # Run all quality checks +``` + +## 🚨 EMERGENCY OVERRIDES + +**Only for critical production issues:** +```yaml +override_label: "EMERGENCY-OVERRIDE" +required_approvers: 2 +max_override_per_week: 1 +auto_close_after: 8h +``` + +## 📊 SUCCESS METRICS + +**Weekly Tracking:** +- Average PR size: < 15 files +- PR lifetime: < 24 hours +- Number of scope violations: 0 +- Merge conflicts: < 1 per week + +**Red Flags (Auto-alert):** +- Any PR > 50 files +- Any branch > 48 hours old +- Any PR title with "and", "also", "plus" +- Any description > 2 sentences + +## 🎯 WHY THIS WORKS + +### **Psychological Benefits** +- **Small wins**: Frequent merges build momentum +- **Fast feedback**: Quick reviews = faster iteration +- **Reduced risk**: Smaller changes = easier rollback +- **Team satisfaction**: Actually shipping features + +### **Technical Benefits** +- **Fewer merge conflicts**: Smaller, focused changes +- **Easier reviews**: 50 files vs 100+ files +- **Better testing**: Isolated changes = targeted tests +- **Faster CI/CD**: Smaller PRs = faster pipelines + +## 🚀 IMPLEMENTATION + +### **Immediate Actions** +1. **Install pre-commit hooks**: `pre-commit install` +2. **Set commit template**: `git config commit.template .gitmessage.txt` +3. **Run scope checker**: `python scripts/check_pr_scope.py` +4. **Review existing PRs**: Close any that violate rules + +### **Team Adoption** +1. **Training session**: Walk through the rules +2. **Documentation**: Share this guide +3. **Examples**: Show good vs bad PR examples +4. **Celebrate**: Recognize teams following the rules + +## 📞 SUPPORT + +**Questions?** Ask in the development channel. + +**Found a violation?** Use the PR comment template: +``` +🚨 **SCOPE VIOLATION DETECTED** +This PR exceeds our size limits: +- Files: [count]/50 max +- Lines: [count]/1500 max +- Multiple concerns: [list them] + +Please split into focused micro-PRs. +``` + +--- + +## 🎉 CONCLUSION + +**Small PRs = Fast reviews = Quick merges = Happy developers = Successful project** + +**NO EXCEPTIONS. NO EXCUSES. NO "JUST THIS ONCE".** + +Welcome to the era of **productive, focused development!** 🚀 From 8fa565c7e865763679b01b1bbe2c46cacf71cc98 Mon Sep 17 00:00:00 2001 From: Deniz Ulker <156104354+uelkerd@users.noreply.github.com> Date: Mon, 22 Sep 2025 23:51:20 +0300 Subject: [PATCH 2/4] fix(docs): update code quality guide based on feedback - Update tooling list to reflect current ruff usage (replaces Black, isort, flake8) - Extend branch lifetime from 48h to 72h with extension clarification - Fix pre-commit framework reference to use .pre-commit-config.yaml - Correct branch naming example from 'feature' to 'feat' for consistency - Clarify PR description rule to encourage detailed body descriptions - Improve accuracy and reduce confusion for developers --- CODE_QUALITY_README.md | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/CODE_QUALITY_README.md b/CODE_QUALITY_README.md index 57e9950c6..68621f191 100644 --- a/CODE_QUALITY_README.md +++ b/CODE_QUALITY_README.md @@ -22,7 +22,7 @@ This document outlines the **strict rules** and **automated enforcement** design max_files_changed: 50 # HARD STOP at 50 files max_lines_changed: 1500 # HARD STOP at 1500 lines max_commits_per_pr: 5 # HARD STOP at 5 commits -branch_lifetime: 48h # FORCE merge or close after 48h +branch_lifetime: 72h # FORCE merge or close after 72h (extensions available for complex changes) ``` ### **Single Purpose Rule** @@ -35,7 +35,7 @@ branch_lifetime: 48h # FORCE merge or close after 48h ## 🛠️ AUTOMATED ENFORCEMENT ### **1. Pre-Commit Hook** -The `.git/hooks/pre-commit` script automatically: +The `pre-commit` framework, configured in `.pre-commit-config.yaml`, automatically runs scripts that: - Validates branch naming (`feat/add-auth`, `fix/memory-leak`) - Checks commit message format (`feat: add user auth`) - Prevents commits with mixed concerns @@ -86,7 +86,7 @@ test/short-description # Test additions - ✅ `fix/validate-input` - ✅ `chore/update-deps` - ✅ `refactor/simplify-logic` -- ❌ `feature/add-auth-and-fix-bugs` *(multiple concerns)* +- ❌ `feat/add-auth-and-fix-bugs` *(multiple concerns)* - ❌ `fix-stuff` *(too vague)* ### **Commit Message Format** @@ -106,9 +106,7 @@ refactor: simplify rate limiter logic ## 🏗️ CODE QUALITY TOOLS ### **Automated Tools** -- **Black**: Code formatting (88 char lines) -- **isort**: Import sorting -- **flake8**: Linting and style +- **Ruff**: Ultra-fast code formatting, linting, and import sorting (replaces Black, isort, flake8) - **pylint**: Advanced code analysis - **mypy**: Type checking - **bandit**: Security scanning @@ -151,7 +149,7 @@ auto_close_after: 8h - Any PR > 50 files - Any branch > 48 hours old - Any PR title with "and", "also", "plus" -- Any description > 2 sentences +- Any PR summary > 2 sentences (detailed descriptions in body are encouraged) ## 🎯 WHY THIS WORKS From 09eb2dded2f231b8ea7a471896b0b18d438ce36d Mon Sep 17 00:00:00 2001 From: Deniz Ulker <156104354+uelkerd@users.noreply.github.com> Date: Mon, 22 Sep 2025 23:53:27 +0300 Subject: [PATCH 3/4] fix(docs): clarify file availability and restore gitmessage template - Add note confirming PR scope checker script availability - Clarify that .gitmessage.txt is included in repository - Restore comprehensive .gitmessage.txt template with Conventional Commits - Address Copilot AI feedback about missing file references - Ensure all referenced files are available and documented --- .gitmessage.txt | 47 ++++++++++++++++++++++++++++++++++++++++++ CODE_QUALITY_README.md | 2 ++ 2 files changed, 49 insertions(+) create mode 100644 .gitmessage.txt diff --git a/.gitmessage.txt b/.gitmessage.txt new file mode 100644 index 000000000..4b97b59d0 --- /dev/null +++ b/.gitmessage.txt @@ -0,0 +1,47 @@ +# SAMO-DL Commit Message Template +# +# Quick setup: +# git config commit.template .gitmessage.txt +# # To make it global: git config --global commit.template "$(pwd)/.gitmessage.txt" +# +# Format: ()!: +# (scope is optional; add "!" for breaking changes) +# Body: explain what and why, wrapped at ~72 chars per line +# Footer: references and BREAKING CHANGE notes +# +# Types: +# feat: A new feature +# fix: A bug fix +# docs: Documentation only changes +# style: Formatting, whitespace, missing semicolons, no code change +# refactor: Code change that neither fixes a bug nor adds a feature +# perf: A code change that improves performance +# test: Adding missing tests or correcting existing tests +# build: Changes to build system or external dependencies +# ci: Changes to CI configuration files and scripts +# chore: Other changes that don't modify src or test files +# revert: Reverts a previous commit +# +# Rules: +# - ONE purpose per commit (no "and", "also", "plus") +# - Subject line ≤ 50 characters +# - Use imperative mood ("Add" not "Added") +# - No period at end of subject line +# - Wrap body at ~72 characters per line +# - Reference issues/PRs in footer (e.g., "Closes #123") +# - Use "BREAKING CHANGE: ..." in footer for breaking changes +# +# Examples: +# feat(auth): add login with email magic links +# fix(loader): resolve memory leak in model loading +# perf(infer): cache tokenizer to reduce setup time +# refactor(rate-limit): simplify token bucket logic +# docs(api): update inference usage examples +# style: run formatter across repo +# build: bump torch to 2.4.x +# ci: parallelize test matrix +# revert: revert "feat(auth): add magic links" +# feat(core)!: switch default precision to bfloat16 +# BREAKING CHANGE: default precision is now bfloat16; set +# SAMO_PRECISION=float32 to keep previous behavior. +# Closes #174 diff --git a/CODE_QUALITY_README.md b/CODE_QUALITY_README.md index 68621f191..b03f9ce4e 100644 --- a/CODE_QUALITY_README.md +++ b/CODE_QUALITY_README.md @@ -45,6 +45,7 @@ The `pre-commit` framework, configured in `.pre-commit-config.yaml`, automatical ```bash python scripts/check_pr_scope.py --strict ``` +> **Note:** The PR scope checker script is available in the repository and actively maintained. Validates: - File count ≤ 50 - Line changes ≤ 1500 @@ -170,6 +171,7 @@ auto_close_after: 8h ### **Immediate Actions** 1. **Install pre-commit hooks**: `pre-commit install` 2. **Set commit template**: `git config commit.template .gitmessage.txt` + > **Note:** The `.gitmessage.txt` file is included in the repository with a comprehensive commit message template following Conventional Commits standards. 3. **Run scope checker**: `python scripts/check_pr_scope.py` 4. **Review existing PRs**: Close any that violate rules From 7d3e358fd06bb96e1c76c24fdaa0681a86d550da Mon Sep 17 00:00:00 2001 From: Deniz Ulker <156104354+uelkerd@users.noreply.github.com> Date: Mon, 22 Sep 2025 23:55:17 +0300 Subject: [PATCH 4/4] fix(docs): address all 9 nitpick comments for better accuracy - Soften 'FORCE merge' language to 'Auto-close or require split' - Fix pre-commit hook wording to reflect framework usage - Correct fence language from bash to md/text/gitcommit - Add proper language tags for all code blocks (MD040) - Fix emphasis as heading (MD036) for better scannability - Clarify override mechanics with GitHub permissions note - Improve documentation accuracy and markdown compliance - Address all linting and formatting issues --- CODE_QUALITY_README.md | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/CODE_QUALITY_README.md b/CODE_QUALITY_README.md index b03f9ce4e..8bf49a0cb 100644 --- a/CODE_QUALITY_README.md +++ b/CODE_QUALITY_README.md @@ -22,11 +22,10 @@ This document outlines the **strict rules** and **automated enforcement** design max_files_changed: 50 # HARD STOP at 50 files max_lines_changed: 1500 # HARD STOP at 1500 lines max_commits_per_pr: 5 # HARD STOP at 5 commits -branch_lifetime: 72h # FORCE merge or close after 72h (extensions available for complex changes) +branch_lifetime: 72h # Auto-close or require split after 72h (extensions available for complex changes) ``` -### **Single Purpose Rule** -**EVERY PR MUST HAVE EXACTLY ONE SENTENCE DESCRIBING ITS PURPOSE** +#### Requirement: PR purpose must be exactly one sentence - ✅ "Add user authentication system" - ✅ "Fix memory leak in model loading" - ❌ "Improve model architecture and fix bugs" *(TWO THINGS!)* @@ -34,12 +33,12 @@ branch_lifetime: 72h # FORCE merge or close after 72h (extensions available ## 🛠️ AUTOMATED ENFORCEMENT -### **1. Pre-Commit Hook** -The `pre-commit` framework, configured in `.pre-commit-config.yaml`, automatically runs scripts that: -- Validates branch naming (`feat/add-auth`, `fix/memory-leak`) -- Checks commit message format (`feat: add user auth`) -- Prevents commits with mixed concerns -- Runs before every commit +### **1. Local Hooks (pre-commit + commit-msg)** +Use the pre-commit framework (versioned via `.pre-commit-config.yaml`) to run format/lint/type/security checks on staged files. Enforce commit message format via a `commit-msg` hook (e.g., Commitizen). + +**Notes:** +- Branch naming and "single purpose" are enforced in CI (see PR Scope Checker), not locally. +- Hooks run automatically once installed (`pre-commit install` and `pre-commit install --hook-type commit-msg`). ### **2. PR Scope Checker** ```bash @@ -61,7 +60,7 @@ GitHub Actions automatically: ## 📋 DEVELOPMENT WORKFLOW ### **Before Creating a Branch** -```bash +```md # Answer these questions: 1. Can I describe this in ONE sentence? 2. Will this affect < 50 files? @@ -73,7 +72,7 @@ GitHub Actions automatically: ``` ### **Branch Naming Convention** -``` +```text feat/short-description # New features fix/short-description # Bug fixes chore/short-description # Build/tooling changes @@ -91,13 +90,13 @@ test/short-description # Test additions - ❌ `fix-stuff` *(too vague)* ### **Commit Message Format** -``` +```gitcommit (): ``` **Examples:** -``` +```gitcommit feat: add JWT token authentication fix: resolve memory leak in model loading chore: update Python dependencies @@ -138,6 +137,8 @@ max_override_per_week: 1 auto_close_after: 8h ``` +> **Note:** Overrides require label + 2 approvers + admin "bypass PR requirements" permission, or a temporary policy change. GitHub cannot bypass required checks by workflow alone. + ## 📊 SUCCESS METRICS **Weekly Tracking:** @@ -186,7 +187,7 @@ auto_close_after: 8h **Questions?** Ask in the development channel. **Found a violation?** Use the PR comment template: -``` +```md 🚨 **SCOPE VIOLATION DETECTED** This PR exceeds our size limits: - Files: [count]/50 max @@ -200,7 +201,8 @@ Please split into focused micro-PRs. ## 🎉 CONCLUSION -**Small PRs = Fast reviews = Quick merges = Happy developers = Successful project** +### Summary +Small PRs = Fast reviews = Quick merges = Happy developers = Successful project **NO EXCEPTIONS. NO EXCUSES. NO "JUST THIS ONCE".**