From f613de7a1a9d23ef3856d3c91318d70b120758c4 Mon Sep 17 00:00:00 2001 From: Zhongwei Li Date: Sun, 30 Nov 2025 08:25:07 +0800 Subject: [PATCH] Initial commit --- .claude-plugin/plugin.json | 12 + LICENSE | 21 + README.md | 3 + SKILL.md | 1233 ++++++++++++++++++++++++++++ assets/bad-pr-example.md | 552 +++++++++++++ assets/good-pr-example.md | 308 +++++++ plugin.lock.json | 81 ++ references/commit-message-guide.md | 534 ++++++++++++ references/files-to-exclude.md | 533 ++++++++++++ references/pr-checklist.md | 351 ++++++++ references/pr-template.md | 266 ++++++ scripts/clean-branch.sh | 190 +++++ scripts/pre-pr-check.sh | 249 ++++++ 13 files changed, 4333 insertions(+) create mode 100644 .claude-plugin/plugin.json create mode 100644 LICENSE create mode 100644 README.md create mode 100644 SKILL.md create mode 100644 assets/bad-pr-example.md create mode 100644 assets/good-pr-example.md create mode 100644 plugin.lock.json create mode 100644 references/commit-message-guide.md create mode 100644 references/files-to-exclude.md create mode 100644 references/pr-checklist.md create mode 100644 references/pr-template.md create mode 100755 scripts/clean-branch.sh create mode 100755 scripts/pre-pr-check.sh diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json new file mode 100644 index 0000000..fd29054 --- /dev/null +++ b/.claude-plugin/plugin.json @@ -0,0 +1,12 @@ +{ + "name": "open-source-contributions", + "description": "Create maintainer-friendly pull requests for open source projects with clean code submissions and professional communication. Prevents 16 common mistakes that cause PR rejection. Use when: contributing to public repositories, submitting PRs to community projects, migrating from contributor to maintainer workflows, or troubleshooting PR rejection, working on main branch errors, failing CI checks, or personal artifacts in commits.", + "version": "1.0.0", + "author": { + "name": "Jeremy Dawes", + "email": "jeremy@jezweb.net" + }, + "skills": [ + "./" + ] +} \ No newline at end of file diff --git a/LICENSE b/LICENSE new file mode 100644 index 0000000..f472709 --- /dev/null +++ b/LICENSE @@ -0,0 +1,21 @@ +MIT License + +Copyright (c) 2025 Jeremy Dawes - Jezweb + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/README.md b/README.md new file mode 100644 index 0000000..bdff369 --- /dev/null +++ b/README.md @@ -0,0 +1,3 @@ +# open-source-contributions + +Create maintainer-friendly pull requests for open source projects with clean code submissions and professional communication. Prevents 16 common mistakes that cause PR rejection. Use when: contributing to public repositories, submitting PRs to community projects, migrating from contributor to maintainer workflows, or troubleshooting PR rejection, working on main branch errors, failing CI checks, or personal artifacts in commits. diff --git a/SKILL.md b/SKILL.md new file mode 100644 index 0000000..ad7c413 --- /dev/null +++ b/SKILL.md @@ -0,0 +1,1233 @@ +--- +name: open-source-contributions +description: | + Create maintainer-friendly pull requests for open source projects with clean code submissions and professional communication. Prevents 16 common mistakes that cause PR rejection. + + Use when: contributing to public repositories, submitting PRs to community projects, migrating from contributor to maintainer workflows, or troubleshooting PR rejection, working on main branch errors, failing CI checks, or personal artifacts in commits. +license: MIT +metadata: + version: "1.1.0" + author: "Jeremy Dawes - Jezweb" + repository: "https://github.com/jezweb/claude-skills" + last_verified: "2025-11-06" + production_tested: true + token_savings: "~70%" + errors_prevented: 16 +--- + +# Open Source Contributions Skill + +**Version**: 1.1.0 | **Last Verified**: 2025-11-06 | **Production Tested**: ✅ + +--- + +## Overview + +Contributing to open source projects requires understanding etiquette, conventions, and what maintainers expect. This skill helps create professional, maintainer-friendly pull requests while avoiding common mistakes that waste time and cause rejections. + +**Key Focus**: Cleaning personal development artifacts, writing proper PR descriptions, following project conventions, and communicating professionally. + +--- + +## When to Use This Skill + +Use this skill when: +- Creating pull requests for public repositories +- Contributing to community open source projects +- Submitting code to projects you don't maintain +- First-time contributor to a new project +- Want to increase PR acceptance rate +- Need guidance on PR best practices + +**Auto-triggers on phrases**: "submit PR to", "contribute to", "pull request for", "open source contribution" + +--- + +## What NOT to Include in Pull Requests + +### Personal Development Artifacts (NEVER Include) + +**Planning & Notes Documents:** +``` +❌ SESSION.md # Session tracking notes +❌ NOTES.md # Personal development notes +❌ TODO.md # Personal todo lists +❌ planning/* # Planning documents directory +❌ IMPLEMENTATION_PHASES.md # Project planning +❌ DATABASE_SCHEMA.md # Unless adding new schema to project +❌ ARCHITECTURE.md # Unless documenting new architecture +❌ SCRATCH.md # Temporary notes +❌ DEBUGGING.md # Debugging notes +❌ research-logs/* # Research notes +``` + +**Screenshots & Visual Assets:** +``` +❌ screenshots/debug-*.png # Debugging screenshots +❌ screenshots/test-*.png # Testing screenshots +❌ screenshot-*.png # Ad-hoc screenshots +❌ screen-recording-*.mp4 # Screen recordings +❌ before-after-local.png # Local comparison images + +✅ screenshots/feature-demo.png # IF demonstrating feature in PR description +✅ docs/assets/ui-example.png # IF part of documentation update +``` + +**Test Files (Situational):** +``` +❌ test-manual.js # Manual testing scripts +❌ test-debug.ts # Debugging test files +❌ quick-test.py # Quick validation scripts +❌ scratch-test.sh # Temporary test scripts +❌ example-local.json # Local test data + +✅ tests/feature.test.js # Proper test suite additions +✅ tests/fixtures/data.json # Required test fixtures +✅ __tests__/component.tsx # Component tests +``` + +**Build & Dependencies:** +``` +❌ node_modules/ # Dependencies (in .gitignore) +❌ dist/ # Build output (in .gitignore) +❌ build/ # Build artifacts (in .gitignore) +❌ .cache/ # Cache files (in .gitignore) +❌ package-lock.json # Unless explicitly required by project +❌ yarn.lock # Unless explicitly required by project +``` + +**IDE & OS Files:** +``` +❌ .vscode/ # VS Code settings +❌ .idea/ # IntelliJ settings +❌ .DS_Store # macOS file system +❌ Thumbs.db # Windows thumbnails +❌ *.swp, *.swo # Vim swap files +❌ *~ # Editor backup files +``` + +**Secrets & Sensitive Data:** +``` +❌ .env # Environment variables (NEVER!) +❌ .env.local # Local environment config +❌ config/local.json # Local configuration +❌ credentials.json # Credentials (NEVER!) +❌ *.key, *.pem # Private keys (NEVER!) +❌ secrets/* # Secrets directory (NEVER!) +``` + +**Temporary & Debug Files:** +``` +❌ temp/* # Temporary files +❌ tmp/* # Temporary directory +❌ debug.log # Debug logs +❌ *.log # Log files +❌ dump.sql # Database dumps +❌ core # Core dumps +❌ *.prof # Profiling output +``` + +### What SHOULD Be Included + +``` +✅ Source code changes # The actual feature/fix +✅ Tests for changes # Required tests for new code +✅ Documentation updates # README, API docs, inline comments +✅ Configuration changes # If part of the feature +✅ Migration scripts # If needed for the feature +✅ Package.json updates # If adding/removing dependencies +✅ Schema changes # If part of feature (with migrations) +✅ CI/CD updates # If needed for new workflows +``` + +--- + +## Pre-PR Cleanup Process + +### Step 1: Run Pre-PR Check Script + +Use the bundled `scripts/pre-pr-check.sh` to scan for artifacts: + +```bash +./scripts/pre-pr-check.sh +``` + +**What it checks:** +- Personal documents (SESSION.md, planning/*, NOTES.md) +- Screenshots not referenced in PR description +- Temporary test files +- Large files (>1MB) +- Potential secrets in file content +- PR size (warns if >400 lines) +- Uncommitted changes + +### Step 2: Review Git Status + +```bash +git status +git diff --stat +``` + +**Ask yourself:** +- Is every file change necessary for THIS feature/fix? +- Are there any unrelated changes? +- Are there files I added during development but don't need? + +### Step 3: Clean Personal Artifacts + +**Manual removal:** +```bash +git rm --cached SESSION.md +git rm --cached -r planning/ +git rm --cached screenshots/debug-*.png +git rm --cached test-manual.js +``` + +**Or use the clean script:** +```bash +./scripts/clean-branch.sh +``` + +### Step 4: Update .gitignore + +Add personal patterns to `.git/info/exclude` (affects only YOUR checkout): +``` +# Personal development artifacts +SESSION.md +NOTES.md +TODO.md +planning/ +screenshots/debug-*.png +test-manual.* +scratch.* +``` + +--- + +## Writing Effective PR Descriptions + +### Use the What/Why/How Structure + +**Template** (see `references/pr-template.md`): + +```markdown +## What? +[Brief description of what this PR does] + +## Why? +[Explain the reasoning, business value, or problem being solved] + +## How? +[Describe the implementation approach and key decisions] + +## Testing +[Step-by-step instructions for reviewers to test] + +## Checklist +- [ ] Tests added/updated +- [ ] Documentation updated +- [ ] CI passing +- [ ] Breaking changes documented + +## Related Issues +Closes #123 +Relates to #456 +``` + +### Example Good PR Description + +```markdown +## What? +Add OAuth2 authentication support for Google and GitHub providers + +## Why? +Users have requested social login to reduce friction during signup. +This implements Key Result 2 of Q4 OKR1. + +## How? +- Implemented OAuth2 flow using passport.js +- Added provider configuration in environment variables +- Created callback routes for each provider +- Updated user model to link social accounts + +## Testing +1. Set up OAuth apps in Google/GitHub developer consoles +2. Add credentials to `.env` (see `.env.example`) +3. Run `npm start` +4. Click "Login with Google" and verify flow +5. Verify user profile merges correctly + +## Breaking Changes +None - this is additive functionality + +## Related Issues +Closes #234 +Relates to #156 (social login epic) +``` + +### Titles: Follow Conventional Commits + +**Format**: `(): ` + +**Types:** +- `feat:` - New feature +- `fix:` - Bug fix +- `docs:` - Documentation only +- `style:` - Formatting (no code change) +- `refactor:` - Code restructuring (no behavior change) +- `perf:` - Performance improvement +- `test:` - Adding/updating tests +- `build:` - Build system changes +- `ci:` - CI configuration changes +- `chore:` - Other changes (no src/test changes) + +**Examples:** +``` +✅ feat(auth): add OAuth2 support for Google and GitHub +✅ fix(api): resolve memory leak in worker shutdown +✅ docs(readme): update installation instructions for v2.0 +✅ refactor(utils): extract validation logic to separate module + +❌ Fixed stuff +❌ Updates +❌ Working on authentication (too vague) +``` + +--- + +## Commit Message Best Practices + +### Structure + +See `references/commit-message-guide.md` for complete guide. + +``` +(): + +[optional body] + +[optional footer] +``` + +**Subject line rules:** +- 50 characters max +- Imperative mood ("Add" not "Added") +- Capitalize first word +- No period at end +- Describe WHAT changed + +**Body rules (optional):** +- 72 characters per line +- Explain WHY (code shows WHAT) +- Separate from subject with blank line + +**Footer (optional):** +- Reference issues: `Closes #123` +- Note breaking changes: `BREAKING CHANGE: ...` + +### Examples + +**Good:** +``` +fix: prevent race condition in cache invalidation + +The cache invalidation logic wasn't thread-safe, causing +occasional race conditions when multiple workers tried to +invalidate the same key simultaneously. + +Added mutex locks around the critical section and tests +to verify thread-safety. + +Fixes #456 +``` + +**Bad:** +``` +Fixed bug # Too vague +WIP # Not descriptive +asdf # Meaningless +Updated auth.js # Describes file, not change +Fixed the issue with the thing # No specifics +``` + +--- + +## PR Sizing Best Practices + +### Ideal Sizes + +**Research-backed guidelines:** +- **Ideal**: 50 lines of code +- **Good**: Under 200 lines +- **Maximum**: 400 lines +- **Beyond 400**: Defect detection drops significantly + +**Why small PRs matter:** +- Faster reviews (maintainers can review in one sitting) +- Easier to find bugs +- Simpler to revert if needed +- Reduced merge conflicts +- More focused discussions + +### How to Keep PRs Small + +**1. One Change = One PR** +``` +❌ BAD: Refactor auth module + add OAuth + fix bug + update docs (800 lines) + +✅ GOOD: + PR #1: Refactor auth module (150 lines) + PR #2: Add OAuth support (200 lines) + PR #3: Fix authentication bug (50 lines) + PR #4: Update authentication docs (80 lines) +``` + +**2. Separate Refactoring from Features** +``` +❌ BAD: Add feature X + refactor related code (500 lines) + +✅ GOOD: + PR #1: Refactor to prepare for feature X (200 lines) + PR #2: Add feature X (150 lines) +``` + +**3. Use Feature Flags** +Ship incomplete features behind flags, merge early and often: +```typescript +if (featureFlags.newAuth) { + // New OAuth flow (incomplete but behind flag) +} else { + // Existing flow +} +``` + +**4. Break Down by Layer** +``` +For "User Authentication" feature: + PR #1: Database schema + migrations (100 lines) + PR #2: API endpoints (150 lines) + PR #3: Frontend components (200 lines) + PR #4: Tests + documentation (150 lines) +``` + +### When Large PRs Are Acceptable + +- Initial project scaffolding +- Generated code (clearly marked as such) +- Large refactors that can't be split (explain why in description) +- Documentation rewrites +- Dependency updates (automated) + +**If unavoidable**: Add clear section headers in PR description to guide review. + +--- + +## Following Project Conventions + +### Step 1: Read CONTRIBUTING.md + +**Always read this FIRST** - it's the project's law. Common locations: +- `/CONTRIBUTING.md` +- `/.github/CONTRIBUTING.md` +- `/docs/CONTRIBUTING.md` + +**Key sections to check:** +- Development setup +- Testing requirements +- Code style guidelines +- Commit message format +- PR process +- Review expectations + +**Tip**: Add `/contribute` to repository URL to find beginner-friendly issues. + +### Step 2: Check for Style Guides + +**Files to look for:** +- `.eslintrc`, `.eslintrc.js` (JavaScript/TypeScript linting) +- `.prettierrc` (Code formatting) +- `.editorconfig` (Editor settings) +- `STYLE.md` (Style guide documentation) +- `rustfmt.toml` (Rust formatting) +- `pyproject.toml` (Python formatting) + +**Run formatters before committing:** +```bash +npm run lint # Check for style issues +npm run lint:fix # Auto-fix style issues +npm run format # Format code (Prettier, Black, etc.) +``` + +### Step 3: Match Existing Patterns + +**Look at recent merged PRs:** +- How are files organized? +- What naming conventions are used? +- How verbose are comments? +- What testing patterns exist? +- How are imports ordered? + +**The Golden Rule**: "Do things the way the project maintainers would want it done." + +### Step 4: Test Requirements + +**Before submitting:** +```bash +# Run full test suite +npm test # Node projects +cargo test # Rust projects +pytest # Python projects +go test ./... # Go projects + +# Check coverage (if required) +npm run test:coverage + +# Run linters +npm run lint + +# Build the project +npm run build +``` + +**Never submit a PR with:** +- Failing tests +- Failing lints +- Build errors +- Untested new code (if tests are required) + +--- + +## Communication Best Practices + +### Before Starting Work + +**Comment on the issue:** +``` +Hi! I'd like to work on this issue. My approach would be to: +1. [Brief outline of approach] +2. [Key implementation detail] + +Does this sound good? Any guidance before I start? +``` + +**Why this matters:** +- Prevents duplicate work +- Gets early feedback on approach +- Shows respect for maintainers' time +- Establishes communication + +**Wait for acknowledgment** on significant changes before investing time. + +### Writing PR Comments + +**When submitting:** +``` +@maintainer I've implemented the feature as discussed in #123. +I went with approach A instead of B because [reason]. + +Ready for review when you have time. Happy to make changes! +``` + +**When responding to feedback:** +``` +Good catch! I've updated this in commit abc1234. +``` + +``` +I considered that approach, but went with X because Y. +Happy to discuss alternatives if you prefer a different direction. +``` + +``` +Could you elaborate on what you mean by Z? +I want to make sure I understand correctly. +``` + +### Responding to Review Comments + +**Key principles:** +1. **Address every comment** - Even if just "Done" or "Acknowledged" +2. **Never respond in anger** - Walk away if needed, respond when calm +3. **Use GitHub's review tools**: + - Mark conversations as resolved when addressed + - Request re-review after changes + - Use "suggestion" feature for quick fixes +4. **Keep discussion on-topic** - Take tangents elsewhere +5. **Be thankful** - Reviews are valuable, time-consuming work + +**Response templates:** +- Implemented: "Good idea! Implemented in [commit hash]" +- Question: "Thanks for asking - I did X because Y. Thoughts?" +- Disagreement: "I see your point. I went with X because Y. Open to alternatives if you feel strongly." +- Clarification needed: "Could you help me understand what you mean by Z?" + +### Handling PR Rejections + +**Mental framework:** +- It's a learning experience, not a personal failure +- Rejection is normal and happens to everyone +- The project's vision might differ from your approach + +**Steps to take:** +1. **Seek clarification** - Ask why the PR was rejected politely +2. **Review feedback carefully** - Identify specific concerns +3. **Learn from it** - What would you do differently next time? +4. **Options**: + - Rework based on feedback + - Fork and maintain your changes + - Move on to other contributions + +**What NOT to do:** +- Don't keep commenting on declined PRs +- Don't take it personally +- Don't argue aggressively +- Don't abandon the project entirely (unless it's truly not a fit) + +### Be Patient (But Know When to Ping) + +**Maintainers are often volunteers:** +- They have day jobs, families, lives +- They may be reviewing dozens of PRs +- They may be dealing with complex project issues + +**Timeline expectations:** +- Small PRs: 1-7 days +- Large PRs: 1-2 weeks +- Complex/controversial: Weeks to months + +**When to ping:** +- After 1 week for small PRs +- After 2 weeks for larger PRs +- When CI is failing due to external changes (rebase needed) + +**How to ping:** +``` +Hi! Just wanted to gently ping this PR. +No rush at all, just making sure it's on your radar. +Happy to answer any questions or make changes! +``` + +--- + +## Common Mistakes That Annoy Maintainers + +### The Top 16 Mistakes + +**See Critical Workflow Rules section for detailed guidance on Rules 1-3** + +**1. Not Reading CONTRIBUTING.md** +- Impact: Wasted effort, rejected PRs, frustrated maintainers +- Fix: ALWAYS read this file first, follow instructions exactly + +**2. Including Personal Development Artifacts** +- Impact: Messy PRs, reveals internal workflow, unprofessional +- Fix: Use pre-PR check script, clean artifacts before submission +- Examples: SESSION.md, planning/*, screenshots, temp test files + +**3. Submitting Massive Pull Requests** +- Impact: Overwhelming to review, harder to find bugs, delays merge +- Fix: Break into smaller PRs (<200 lines ideal) + +**4. Not Testing Code Before Submitting (Violates RULE 2)** +- Impact: CI failures, bugs in production, maintainer has to fix, loss of trust +- Fix: See **RULE 2** - Run full test suite, test manually, capture evidence (screenshots/videos), verify CI will pass + +**5. Working on Already Assigned Issues** +- Impact: Duplicate work, wasted time for both contributors +- Fix: Check issue assignments, comment to claim work + +**6. Not Discussing Changes First (for large changes)** +- Impact: Unwanted features, rejected PRs, wasted development time +- Fix: Open issue or comment to discuss before coding + +**7. Being Impatient or Unresponsive** +- Impact: PRs stall, maintainers move on, contributions abandoned +- Fix: Be responsive but patient, ping respectfully after 1-2 weeks + +**8. Not Updating Documentation** +- Impact: Users and developers confused by undocumented changes +- Fix: Update README, API docs, inline comments as needed + +**9. Ignoring Code Style Standards** +- Impact: Messy codebase, maintainer has to fix formatting +- Fix: Use project's linters/formatters, match existing style + +**10. Ignoring CI Failures** +- Impact: Can't merge, blocks progress, maintainer has to investigate +- Fix: Monitor CI, fix failures immediately, ask for help if stuck + +**11. Including Unrelated Changes (Violates RULE 3)** +- Impact: Difficult review, harder to revert, scope creep, confusing git history +- Fix: See **RULE 3** - One PR = One Feature. Keep focused, split unrelated changes into separate branches/PRs + +**12. Not Linking Issues Properly** +- Impact: Lost context, manual issue closing, harder to track +- Fix: Use "Closes #123" or "Fixes #456" in PR description + +**13. Committing Secrets or Sensitive Data** +- Impact: Security risk, requires emergency response, potential breach +- Fix: Use .gitignore, scan for secrets, never commit .env files + +**14. Force-Pushing Without Warning** +- Impact: Breaks reviewer's local copies, loses comments +- Fix: Avoid force-push after review starts, warn if necessary + +**15. Not Running Build/Tests Locally** +- Impact: CI failures for obvious issues, wastes CI resources +- Fix: Always run `npm run build` and `npm test` before pushing + +**16. Working Directly on main/master Branch (Violates RULE 1)** +- Impact: Messy git history, conflicts with upstream, can't work on multiple features, difficult to sync fork +- Fix: See **RULE 1** - ALWAYS create feature branches (`git checkout -b feature/my-feature`), NEVER commit to main + +--- + +## GitHub-Specific Best Practices + +### Fork Workflow (Standard for Open Source) + +**Initial setup:** +```bash +# 1. Fork repository on GitHub (click Fork button) + +# 2. Clone YOUR fork +git clone https://github.com/YOUR-USERNAME/project-name.git +cd project-name + +# 3. Add upstream remote +git remote add upstream https://github.com/ORIGINAL-OWNER/project-name.git + +# 4. Verify remotes +git remote -v +# origin https://github.com/YOUR-USERNAME/project-name.git (fetch) +# origin https://github.com/YOUR-USERNAME/project-name.git (push) +# upstream https://github.com/ORIGINAL-OWNER/project-name.git (fetch) +# upstream https://github.com/ORIGINAL-OWNER/project-name.git (push) +``` + +**Creating feature branch:** +```bash +# NEVER work on main/master directly! +git checkout -b feature/add-oauth-support + +# Make changes... +git add . +git commit -m "feat(auth): add OAuth2 support" + +# Push to YOUR fork +git push origin feature/add-oauth-support +``` + +**Keeping fork in sync:** +```bash +git fetch upstream +git checkout main +git merge upstream/main +git push origin main +``` + +--- + +### Critical Workflow Rules (NEVER SKIP) + +**RULE 1: ALWAYS Work on a Feature Branch** + +```bash +# ❌ NEVER DO THIS +git checkout main +# make changes directly on main +git commit -m "add feature" # BAD! + +# ✅ ALWAYS DO THIS +git checkout main +git pull upstream main # Sync first +git checkout -b feature/add-oauth-support # Descriptive branch name +# make changes on feature branch +git commit -m "feat(auth): add OAuth support" +``` + +**Why this matters:** +- Keeps main clean and in sync with upstream +- Allows working on multiple features simultaneously +- Makes it easy to abandon or restart work +- Required by most open source projects +- Prevents conflicts and messy git history + +**Branch naming conventions:** +``` +feature/descriptive-name # New features +fix/issue-123 # Bug fixes +docs/update-readme # Documentation +refactor/extract-utils # Refactoring +test/add-unit-tests # Test additions +``` + +--- + +**RULE 2: Test Thoroughly BEFORE Submitting PR** + +Never submit a PR without: + +1. **Running the full test suite locally:** + ```bash + npm test # Node.js + cargo test # Rust + pytest # Python + go test ./... # Go + ``` + +2. **Testing manually** (hands-on verification): + - Run the application + - Test the specific feature you added + - Test edge cases and error conditions + - Verify it works as intended + +3. **Capturing evidence** (for visual/UI changes): + - Take screenshots showing the feature working + - Record a short demo video if complex + - Include in PR description to demonstrate functionality + +4. **Checking CI will pass:** + ```bash + npm run lint # Linting + npm run build # Build succeeds + npm run type-check # TypeScript checks + ``` + +**Example testing checklist for a new OAuth feature:** +```markdown +## Testing Performed + +### Automated Tests +- ✅ All existing tests pass +- ✅ Added 12 new tests for OAuth flow +- ✅ Coverage increased from 85% to 87% + +### Manual Testing +- ✅ Tested Google OAuth flow end-to-end +- ✅ Tested GitHub OAuth flow end-to-end +- ✅ Verified error handling (invalid tokens, network failures) +- ✅ Confirmed user profile merging works correctly +- ✅ Tested on Chrome, Firefox, Safari + +### Evidence +See screenshots below showing successful login flow. +``` + +**Why this matters:** +- Prevents CI failures that waste maintainer time +- Shows you care about code quality +- Demonstrates the feature actually works +- Builds trust with maintainers +- Catches bugs before review + +**Screenshots for PR descriptions:** +``` +✅ Include: Working feature demonstration +✅ Include: Before/after comparisons for changes +✅ Include: Error state handling + +❌ Don't include: Debug screenshots in code commits +❌ Don't include: Personal testing screenshots in repo +❌ Don't include: Unrelated screenshots + +Location: Add to PR description on GitHub, NOT as files in commits +``` + +--- + +**RULE 3: Keep PRs Focused and Cohesive** + +**One PR = One Feature/Fix** + +``` +✅ GOOD: Single PR adding OAuth support + - OAuth provider configuration + - Login/callback routes + - User model updates + - Tests for OAuth flow + - Documentation for setup + +❌ BAD: PR with multiple unrelated changes + - OAuth support + - + Unrelated bug fix in validation + - + Random formatting changes + - + Update to README for different feature +``` + +**Why cohesive PRs matter:** +- Easier to review and understand +- Can be merged/rejected independently +- Simpler to revert if issues found +- Faster review cycles +- Clear git history + +**How to keep PRs focused:** + +1. **Plan before coding:** + - What is the ONE thing this PR does? + - What files MUST be changed for this feature? + - What's the minimal set of changes needed? + +2. **During development:** + - If you notice an unrelated bug, create a separate branch + - If you're tempted to fix formatting, do it in a separate PR + - Stay focused on the single goal + +3. **Before committing:** + - Review your changes: `git diff` + - Ask: "Is this file change necessary for THIS feature?" + - Move unrelated changes to stash: `git stash` + +4. **Size guidelines:** + - Ideal: <200 lines changed + - Acceptable: 200-400 lines + - Large: 400-800 lines (needs justification) + - Too large: >800 lines (should be split) + +**Breaking up large features:** + +If a feature is naturally large, break it into phases: + +``` +PR #1: Database schema and models +PR #2: API endpoints +PR #3: Frontend components +PR #4: Integration and tests +``` + +Each PR: +- Builds on the previous +- Is independently reviewable +- Adds working functionality (or is clearly marked as WIP/foundational) + +**Handling accidental scope creep:** + +```bash +# You added OAuth + accidentally fixed a bug +# Split them into separate commits and PRs: + +# 1. Create bug fix branch from main +git checkout main +git checkout -b fix/validation-bug + +# 2. Cherry-pick just the bug fix commits +git cherry-pick + +# 3. Submit bug fix PR separately +git push origin fix/validation-bug + +# 4. Continue with OAuth PR +git checkout feature/add-oauth-support +# Remove bug fix commits if needed +git rebase -i main +``` + +--- + +### Using Draft PRs + +**When to use:** +- Code is work-in-progress, not ready for review +- You want early feedback on approach +- Starting conversation about implementation +- Sharing incomplete code for collaboration + +**Benefits:** +- Prevents accidental merging +- Suppresses reviewer notifications +- Clearly signals "not ready yet" + +**Create draft PR:** +```bash +gh pr create --draft --title "WIP: Add OAuth support" +``` + +**Mark ready when:** +- Code is complete and tested +- CI is passing +- Documentation is updated +- Ready for formal review + +```bash +gh pr ready +``` + +### Linking Issues Properly + +**Automatic closing keywords** (in PR description or commits): +```markdown +Closes #123 +Fixes #456 +Resolves #789 + +# Also works: +Close #123 +Fix #123 +Resolve #123 + +# Case insensitive: +CLOSES #123 +fixes #123 + +# Multiple issues: +Fixes #10, closes #20, resolves #30 + +# Cross-repo: +Fixes owner/repo#123 +``` + +**Important**: Only works when merging to DEFAULT branch (usually main). + +### Using GitHub CLI + +**Creating PRs efficiently:** +```bash +# Interactive (prompts for title/body) +gh pr create + +# Auto-fill from commits +gh pr create --fill + +# Complete non-interactive +gh pr create \ + --title "feat(auth): add OAuth support" \ + --body "$(cat <<'EOF' +## What? +Add OAuth2 authentication + +## Why? +User request for social login + +## Testing +1. Set up OAuth apps +2. Run npm start +3. Test login flow +EOF +)" + +# With reviewers +gh pr create --reviewer username1,username2 + +# Draft PR +gh pr create --draft +``` + +**Other useful commands:** +```bash +gh pr status # See your PRs +gh pr checks # View CI status +gh pr view # View current PR +gh pr diff # See PR diff +gh pr ready # Mark draft as ready +gh pr merge # Merge PR (when approved) +``` + +--- + +## Pre-Submission Checklist + +Use this checklist before submitting ANY pull request (see `references/pr-checklist.md`). + +### Pre-Contribution + +- [ ] Read CONTRIBUTING.md thoroughly +- [ ] Read CODE_OF_CONDUCT.md if present +- [ ] Checked if issue should be created first +- [ ] Commented on issue to claim work +- [ ] Maintainer acknowledged work (for significant changes) +- [ ] Forked repository (if no write access) +- [ ] Set up upstream remote +- [ ] Created feature branch (NEVER work on main) + +### Development + +**Critical Workflow Rules** (see detailed section above): +- [ ] **RULE 1**: Working on feature branch (NOT main/master) +- [ ] **RULE 2**: Tested thoroughly with evidence ready +- [ ] **RULE 3**: PR is focused on single feature/fix + +**Code Quality:** +- [ ] Code follows project style guidelines +- [ ] Ran linters and formatters +- [ ] All tests pass locally (`npm test`, `cargo test`, etc.) +- [ ] Added tests for new functionality +- [ ] Manual testing performed (run app, test feature) +- [ ] Evidence captured (screenshots/videos for visual changes) +- [ ] Updated relevant documentation +- [ ] Commit messages follow conventions +- [ ] No merge conflicts with base branch +- [ ] Built project successfully (`npm run build`, etc.) + +### Cleanup + +- [ ] Ran pre-PR check script (`./scripts/pre-pr-check.sh`) +- [ ] Removed personal artifacts: + - [ ] No SESSION.md or notes files + - [ ] No planning/* documents + - [ ] No debug screenshots + - [ ] No temporary test files + - [ ] No personal workflow files +- [ ] Reviewed git status for unwanted files +- [ ] No secrets or sensitive data +- [ ] No IDE/OS-specific files +- [ ] No build artifacts or dependencies + +### PR Quality + +- [ ] PR is focused on one change/issue +- [ ] PR is reasonably sized (<200 lines ideal, <400 max) +- [ ] No unrelated changes included +- [ ] Reviewed own diff for accidental changes +- [ ] Title follows conventions (Conventional Commits) +- [ ] Description uses What/Why/How structure +- [ ] Testing instructions included +- [ ] Links to related issues (Closes #123) +- [ ] Screenshots for visual changes (if demonstrating feature) +- [ ] Breaking changes noted +- [ ] Reviewers can understand without asking questions + +### Submission + +- [ ] CI/checks passing +- [ ] Pushed to feature branch on fork +- [ ] Created PR with proper description +- [ ] Assigned labels (if permitted) +- [ ] Requested reviewers (if known) +- [ ] Linked to project/milestone (if applicable) + +### Post-Submission + +- [ ] Monitoring CI for failures +- [ ] Responsive to feedback +- [ ] Addressing review comments promptly +- [ ] Marking conversations as resolved +- [ ] Requesting re-review after changes +- [ ] Thanking reviewers +- [ ] Being patient while waiting + +--- + +## Quick Command Reference + +### Git Workflow + +```bash +# Setup +git clone https://github.com/YOUR-USERNAME/project.git +git remote add upstream https://github.com/ORIGINAL/project.git + +# Create feature branch +git checkout -b feature/my-feature + +# Make changes and commit +git add . +git commit -m "feat: add new feature" + +# Push to your fork +git push origin feature/my-feature + +# Keep fork synced +git fetch upstream +git checkout main +git merge upstream/main +git push origin main + +# Rebase feature branch if needed +git checkout feature/my-feature +git rebase main +git push origin feature/my-feature --force-with-lease +``` + +### GitHub CLI + +```bash +# Create PR +gh pr create --title "feat: ..." --body "..." + +# Check status +gh pr status +gh pr checks + +# View PR +gh pr view +gh pr diff + +# Manage PR +gh pr ready # Mark draft as ready +gh pr review --approve # Approve (if reviewer) +gh pr merge # Merge (when approved) +``` + +### Pre-PR Validation + +```bash +# Run checks +npm run lint +npm test +npm run build + +# Use skill script +./scripts/pre-pr-check.sh + +# Check git status +git status +git diff --stat + +# Review changes +git diff +git log --oneline -5 +``` + +--- + +## Examples + +See bundled examples for complete before/after comparisons: +- `assets/good-pr-example.md` - Well-structured PR +- `assets/bad-pr-example.md` - Common mistakes to avoid + +--- + +## Key Takeaways + +1. **Clean Your PRs** - Remove all personal development artifacts before submission +2. **Read CONTRIBUTING.md** - Always read this first, follow instructions exactly +3. **Keep PRs Small** - <200 lines ideal, one change per PR +4. **Test Everything** - Locally before submitting, fix CI failures immediately +5. **Follow Conventions** - Style, commits, testing, documentation +6. **Communicate Well** - Be respectful, responsive, patient, and professional +7. **Link Issues** - Use "Closes #123" to auto-close related issues +8. **Be Patient** - Maintainers are often volunteers with limited time +9. **Learn from Feedback** - Every review is a learning opportunity +10. **Use Automation** - Scripts, templates, and checklists prevent mistakes + +--- + +## Resources + +**Bundled Resources:** +- `scripts/pre-pr-check.sh` - Scan for artifacts before submission +- `scripts/clean-branch.sh` - Remove common personal artifacts +- `references/pr-template.md` - PR description template +- `references/pr-checklist.md` - Complete pre-submission checklist +- `references/commit-message-guide.md` - Conventional commits guide +- `references/files-to-exclude.md` - Comprehensive exclusion list +- `assets/good-pr-example.md` - Example of well-structured PR +- `assets/bad-pr-example.md` - Common mistakes to avoid + +**External Resources:** +- GitHub Open Source Guides: https://opensource.guide/ +- How to Contribute: https://opensource.guide/how-to-contribute/ +- Conventional Commits: https://www.conventionalcommits.org/ +- GitHub CLI: https://cli.github.com/manual/ + +--- + +**Production Tested**: ✅ Based on real-world open source contributions and maintainer feedback + +**Token Efficiency**: ~70% savings vs learning through trial-and-error + +**Errors Prevented**: 15 common mistakes documented with solutions + +**Last Verified**: 2025-11-05 diff --git a/assets/bad-pr-example.md b/assets/bad-pr-example.md new file mode 100644 index 0000000..05b3d32 --- /dev/null +++ b/assets/bad-pr-example.md @@ -0,0 +1,552 @@ +# Bad PR Example + +This example demonstrates common mistakes that annoy maintainers and lead to PR rejection or delays. Learn what NOT to do! + +--- + +## Example: Adding OAuth2 Authentication (Done Wrong) + +### PR Title ❌ +``` +Updated code +``` + +**Why it's bad:** +- ❌ Too vague +- ❌ Doesn't describe what changed +- ❌ No type prefix (feat/fix/docs) +- ❌ Wrong tense (past instead of imperative) +- ❌ Lowercase (should capitalize) + +**Should be:** +``` +feat(auth): add OAuth2 support for Google and GitHub providers +``` + +--- + +### PR Description ❌ + +```markdown +## Changes +Added OAuth + +## Testing +Works for me +``` + +**Why it's bad:** +- ❌ No explanation of WHAT was added +- ❌ No explanation of WHY it's needed +- ❌ No explanation of HOW it works +- ❌ No testing instructions for reviewers +- ❌ No issue links +- ❌ No screenshots +- ❌ "Works for me" is not helpful + +**Result:** Maintainer has to ask multiple questions before even starting review. + +--- + +### Files Changed (Messy!) ❌ + +``` +.env # ❌ SECRETS COMMITTED! +.vscode/settings.json # ❌ Personal IDE config +SESSION.md # ❌ Personal development notes +planning/oauth-implementation.md # ❌ Planning documents +screenshots/debug-oauth-1.png # ❌ Debug screenshots +screenshots/debug-oauth-2.png # ❌ Debug screenshots +screenshots/test-local.png # ❌ Personal testing screenshots +test-manual.js # ❌ Temporary test file +NOTES.md # ❌ Personal notes +TODO.md # ❌ Personal todo list +node_modules/ # ❌ Dependencies (should be in .gitignore!) +package-lock.json # ❌ Lock file changes (not needed) +src/routes/auth.js # ✅ Actual feature code +src/routes/users.js # ❌ Unrelated changes +src/routes/posts.js # ❌ Unrelated changes +src/components/Header.js # ❌ Unrelated changes +src/utils/formatting.js # ❌ Unrelated refactoring +README.md # ✅ Documentation update +docs/authentication.md # ✅ OAuth documentation +``` + +**Problems:** +- ❌ **28 files changed** (way too many!) +- ❌ **Secrets committed** (.env file with API keys) +- ❌ **Personal artifacts** (SESSION.md, NOTES.md, planning/) +- ❌ **Unrelated changes** (users.js, posts.js, Header.js) +- ❌ **Mixed concerns** (feature + refactoring + bug fixes) + +**Should be:** 5-8 files, only those directly related to OAuth feature. + +--- + +### Commit History (Terrible!) ❌ + +``` +WIP +fixed stuff +asdf +more changes +fix +actually working now +Final commit +Actually final +OK this one is final +oops forgot something +``` + +**Why it's bad:** +- ❌ 10 commits for one feature (should squash) +- ❌ Meaningless commit messages +- ❌ "WIP" commits (not production-ready) +- ❌ No explanation of what changed +- ❌ No issue links +- ❌ Commit history shows trial-and-error (should clean up) + +**Should be:** 1-2 clean commits with proper messages. + +--- + +### SESSION.md Content (Should Never Be Committed!) ❌ + +```markdown +# OAuth Implementation Session + +## 2025-11-04 +Started working on OAuth. Not sure if I should use passport or just implement it myself. Going to try passport first. + +## 2025-11-05 +Passport is confusing. Spent 3 hours debugging. Finally got it working but the code is messy. + +TODO: +- Clean up the callback logic +- Add tests (maybe) +- Figure out how to handle errors +- Ask @maintainer about rate limiting? + +NOTES: +- Google OAuth app ID: 123456789-abcdefgh.apps.googleusercontent.com +- Redirect URI: http://localhost:3000/auth/google/callback +- Remember to add to staging: https://staging.myapp.com/auth/google/callback + +BUGS FOUND: +- Header component has alignment issue on mobile +- Post creation form doesn't validate correctly +- User profile page crashes when avatar is null +``` + +**Why this should NEVER be in a PR:** +- ❌ Personal development journal +- ❌ Reveals your confusion/struggle +- ❌ Contains unfinished TODOs +- ❌ Mentions unrelated bugs +- ❌ Unprofessional appearance +- ❌ Pollutes the project + +--- + +### NOTES.md Content (Should Never Be Committed!) ❌ + +```markdown +# Development Notes + +## OAuth Research +Looked at how GitHub and GitLab do OAuth. Their implementations are pretty complex. Mine is simpler. + +## Design Decisions +- Using passport because it's easier +- Not implementing token refresh yet (can do later) +- Rate limiting - should probably add this but skipping for now +- Testing - added some tests but not complete coverage + +## Things I'm Not Sure About +- Is the error handling good enough? +- Should I use sessions or JWT? +- Do I need to validate the email from OAuth providers? + +## Known Issues +- Doesn't work in Safari (CORS issue) +- Memory leak in callback handler (need to fix) +- Missing rate limiting (security risk?) +``` + +**Why this hurts your PR:** +- ❌ Shows incomplete work +- ❌ Admits to known issues not mentioned in PR +- ❌ Reveals security concerns not addressed +- ❌ Makes maintainer lose confidence +- ❌ Creates more work for maintainer + +--- + +### Communication During Review ❌ + +**Initial Comment:** +``` +Here's my OAuth implementation. Let me know what you think. +``` + +**Why it's bad:** +- ❌ No context +- ❌ No explanation +- ❌ No testing instructions +- ❌ Sounds careless + +--- + +**Response to Feedback (Poor):** + +**Maintainer:** "Could you add tests for the error cases?" + +**Bad Response:** +``` +Tests are boring. The code works fine without them. +``` + +**Why it's bad:** +- ❌ Dismissive +- ❌ Unprofessional +- ❌ Doesn't follow project standards +- ❌ Shows lack of respect + +--- + +**Maintainer:** "This PR is too large. Could you split it into smaller PRs?" + +**Bad Response:** +``` +It's all related though. I don't want to spend time splitting it up. +``` + +**Why it's bad:** +- ❌ Refuses reasonable request +- ❌ Doesn't consider reviewer's time +- ❌ Makes review harder +- ❌ Likely to be closed + +--- + +**No Response for 2 Weeks** ❌ + +**Maintainer:** "The tests are failing. Can you fix them?" + +**Your response:** [Silence for 2 weeks] + +**Why it's bad:** +- ❌ Wastes maintainer's time +- ❌ PR goes stale +- ❌ Likely to be closed +- ❌ Damages your reputation + +--- + +### PR Metrics ❌ + +**Size:** +- Lines changed: 847 +- Files changed: 28 +- Commits: 10 + +**Problems:** +- ❌ Way too large (should be <200 lines) +- ❌ Too many files (includes unrelated changes) +- ❌ Messy commit history + +**Timeline:** +- Submitted: Day 1 +- Maintainer requests changes: Day 2 +- No response: Days 3-16 +- PR closed as stale: Day 17 + +--- + +## Specific Mistakes Breakdown + +### 1. Committed Secrets ❌ + +**.env file contents:** +``` +GOOGLE_CLIENT_ID=123456789-abcdefgh.apps.googleusercontent.com +GOOGLE_CLIENT_SECRET=GOCSPX-abc123def456 # ❌ SECRET! +GITHUB_CLIENT_ID=Iv1.1234567890abcdef +GITHUB_CLIENT_SECRET=1234567890abcdef1234567890abcdef12345678 # ❌ SECRET! +DATABASE_URL=postgresql://admin:SuperSecret123@localhost/myapp # ❌ PASSWORD! +``` + +**Impact:** +- 🚨 Security breach! +- 🚨 Must rotate all secrets immediately +- 🚨 Potentially compromises production +- 🚨 Even if removed later, it's in git history + +**What you should have done:** +- ✅ Only commit .env.example with placeholder values +- ✅ Add .env to .gitignore +- ✅ Never commit actual secrets + +--- + +### 2. Including Unrelated Changes ❌ + +**src/routes/users.js:** +```javascript +// OAuth PR includes this "drive-by fix" +exports.getUser = async (req, res) => { + const user = await User.findById(req.params.id); + // Fixed bug where avatar URL was broken + if (user.avatar) { + user.avatar = user.avatar.replace('http://', 'https://'); + } + res.json(user); +}; +``` + +**Why it's bad:** +- ❌ Not related to OAuth feature +- ❌ Makes PR harder to review +- ❌ Mixed concerns +- ❌ If PR is reverted, this fix goes too + +**What you should have done:** +- ✅ Create separate PR for bug fix +- ✅ Keep OAuth PR focused on OAuth only + +--- + +### 3. Massive PR with No Breakdown ❌ + +**Includes all at once:** +- OAuth implementation (200 lines) +- Refactoring of existing auth (300 lines) +- Bug fixes in unrelated files (150 lines) +- UI updates (100 lines) +- Test updates (97 lines) + +**Total: 847 lines in 28 files** + +**Why it's bad:** +- ❌ Takes hours to review +- ❌ Hard to find bugs +- ❌ Difficult to discuss +- ❌ Can't merge incrementally + +**What you should have done:** +- ✅ PR #1: Refactor existing auth (300 lines) +- ✅ PR #2: Add OAuth backend (200 lines) +- ✅ PR #3: Add OAuth UI (100 lines) +- ✅ PR #4: Fix related bugs (150 lines) + +--- + +### 4. Poor Testing ❌ + +**test-manual.js (Committed by mistake!):** +```javascript +// Quick test script - DELETE BEFORE COMMIT! +const axios = require('axios'); + +async function testOAuth() { + // This only works on my machine + const response = await axios.get('http://localhost:3000/auth/google'); + console.log('Works!'); +} + +testOAuth(); +``` + +**Why it's bad:** +- ❌ Not a proper test +- ❌ Hardcoded localhost +- ❌ No assertions +- ❌ Comment says "DELETE BEFORE COMMIT" +- ❌ Clutters project + +**What you should have done:** +- ✅ Delete this file +- ✅ Write proper tests in tests/auth/oauth.test.js +- ✅ Use project's testing framework +- ✅ Include assertions and edge cases + +--- + +### 5. Missing Documentation ❌ + +**README.md changes:** +```markdown +## Authentication + +We now have OAuth. +``` + +**Why it's bad:** +- ❌ No setup instructions +- ❌ No explanation of how it works +- ❌ No examples +- ❌ Unhelpful to users + +**Should be:** +```markdown +## Authentication + +### OAuth 2.0 Social Login + +Users can sign in with Google or GitHub accounts. + +#### Setup + +1. Create OAuth apps: + - Google: https://console.cloud.google.com/apis/credentials + - GitHub: https://github.com/settings/developers + +2. Add credentials to `.env`: + ``` + GOOGLE_CLIENT_ID=your_client_id + GOOGLE_CLIENT_SECRET=your_client_secret + GITHUB_CLIENT_ID=your_client_id + GITHUB_CLIENT_SECRET=your_client_secret + ``` + +3. Restart server + +#### Usage + +Users will see "Sign in with Google" and "Sign in with GitHub" buttons on the login page. + +For detailed implementation, see [docs/authentication.md](docs/authentication.md). +``` + +--- + +### 6. Ignoring CI Failures ❌ + +**CI Status:** +``` +❌ Tests: 5 failing +❌ Lint: 23 errors +❌ Build: Failed +``` + +**Your response:** Submit PR anyway, hope maintainer doesn't notice + +**Why it's bad:** +- ❌ Shows you didn't test +- ❌ Wastes CI resources +- ❌ Can't merge with failing CI +- ❌ Unprofessional + +**What you should have done:** +- ✅ Fix all CI issues before submitting +- ✅ Run tests locally first: `npm test` +- ✅ Run lint locally: `npm run lint` +- ✅ Only submit when all checks pass + +--- + +## The Result + +**Maintainer's Response:** +``` +Thanks for the PR, but there are several issues: + +1. You've committed secrets in the .env file - this is a security issue +2. The PR includes unrelated changes (users.js, posts.js, Header.js) +3. Personal development files (SESSION.md, NOTES.md, planning/) shouldn't be here +4. The PR is too large - 847 lines across 28 files +5. Tests are failing +6. Missing proper documentation +7. test-manual.js is committed + +Please: +- Remove all secrets and rotate them +- Create separate PRs for unrelated changes +- Remove personal development artifacts +- Fix the failing tests +- Add proper documentation +- Keep the PR focused on OAuth only + +I'm closing this for now. Please feel free to submit a new PR addressing these issues. +``` + +**Status:** ❌ PR Closed + +--- + +## Key Lessons + +### What Went Wrong + +1. **Security**: Committed secrets (.env file) +2. **Scope**: Too large, too many unrelated changes +3. **Artifacts**: Personal files committed (SESSION.md, NOTES.md) +4. **Testing**: Poor testing, CI failures +5. **Documentation**: Inadequate documentation +6. **Communication**: Poor responses to feedback +7. **Quality**: Messy commits, no code review +8. **Professionalism**: Dismissive attitude + +### How to Fix It + +1. **Security** + - Never commit secrets + - Use .env.example with placeholders + - Run pre-PR check script + +2. **Scope** + - Keep PRs small (<200 lines) + - One feature per PR + - No unrelated changes + +3. **Artifacts** + - Remove SESSION.md, NOTES.md, TODO.md + - Remove planning/ directory + - Remove debug screenshots + - Use clean-branch script + +4. **Testing** + - Write proper tests + - Fix CI before submitting + - Test locally first + +5. **Documentation** + - Update README + - Add setup instructions + - Include examples + +6. **Communication** + - Be responsive + - Be respectful + - Accept feedback gracefully + +7. **Quality** + - Clean commit history + - Proper commit messages + - Review your own code + +8. **Professionalism** + - Respect maintainer's time + - Follow project conventions + - Be patient and courteous + +--- + +## Comparison: Bad vs Good + +| Aspect | Bad PR ❌ | Good PR ✅ | +|--------|-----------|-----------| +| **Title** | "Updated code" | "feat(auth): add OAuth2 support" | +| **Size** | 847 lines, 28 files | 180 lines, 9 files | +| **Commits** | 10 messy commits | 1 clean commit | +| **Files** | Includes SESSION.md, .env | Only relevant files | +| **Testing** | test-manual.js, CI failing | Proper tests, CI passing | +| **Docs** | "We now have OAuth" | Complete setup guide | +| **Secrets** | Committed .env | Only .env.example | +| **Scope** | OAuth + bugs + refactor | OAuth only | +| **Communication** | Dismissive | Professional | +| **Result** | Closed | Merged in 3 days | + +--- + +**Remember:** Every mistake in this example is based on real PRs that have been rejected. Learn from these errors and follow the good PR example instead! diff --git a/assets/good-pr-example.md b/assets/good-pr-example.md new file mode 100644 index 0000000..722b0de --- /dev/null +++ b/assets/good-pr-example.md @@ -0,0 +1,308 @@ +# Good PR Example + +This example demonstrates a well-structured pull request that follows best practices for open source contributions. + +--- + +## Example: Adding OAuth2 Authentication + +### PR Title +``` +feat(auth): add OAuth2 support for Google and GitHub providers +``` + +**Why it's good:** +- ✅ Uses Conventional Commits format (feat) +- ✅ Includes scope (auth) +- ✅ Clear, specific description +- ✅ Under 50 characters +- ✅ Imperative mood + +--- + +### PR Description + +```markdown +## What? +Add OAuth2 authentication support for Google and GitHub providers, allowing users to sign in with their social accounts. + +## Why? +Multiple users have requested social login to reduce friction during signup (issues #234, #156). This addresses a key pain point: 40% of attempted signups are abandoned at the password creation step according to our analytics. + +This implements Key Result 2 of Q4 OKR1: "Reduce signup friction by 30%" + +## How? +- Implemented OAuth2 flow using passport.js strategy pattern +- Added provider configuration via environment variables (no hardcoded secrets) +- Created callback routes for each provider: + - `/auth/google/callback` + - `/auth/github/callback` +- Updated user model to link social accounts with existing email-based accounts +- Added middleware to automatically merge accounts if user already exists with same email +- Implemented proper error handling for failed OAuth attempts + +## Testing + +### Setup +1. Create OAuth apps in developer consoles: + - Google: https://console.cloud.google.com/apis/credentials + - GitHub: https://github.com/settings/developers +2. Add credentials to `.env` file (see `.env.example` for required variables) +3. Run `npm install` to ensure passport dependencies are installed + +### Manual Testing Steps +1. Start server: `npm start` +2. Navigate to `http://localhost:3000/login` +3. Click "Login with Google" button +4. Verify OAuth flow redirects to Google consent screen +5. Grant permissions +6. Verify redirect back to app with user logged in +7. Check user profile shows data from Google (name, email, avatar) +8. Repeat steps 3-7 for GitHub provider + +### Test Cases Covered +- ✅ New user signup via OAuth +- ✅ Existing user login via OAuth +- ✅ Account merging (same email, different provider) +- ✅ Error handling (user denies permissions) +- ✅ Error handling (OAuth provider timeout) +- ✅ Security: CSRF token validation + +### Automated Tests +```bash +npm test -- tests/auth/oauth.test.js +``` + +All 15 test cases pass, including edge cases. + +## Checklist +- [x] Tests added/updated (tests/auth/oauth.test.js) +- [x] Documentation updated: + - [x] README.md (setup instructions) + - [x] docs/authentication.md (OAuth flow documentation) + - [x] .env.example (required environment variables) +- [x] CI passing (all checks green) +- [x] No breaking changes +- [x] Follows existing code style +- [x] Security review completed (no secrets committed) + +## Related Issues +Closes #234 +Relates to #156 (social login epic) + +## Screenshots + +### Login Page with OAuth Buttons +![OAuth Login Buttons](./screenshots/oauth-buttons.png) +*New social login buttons integrated into existing login page* + +### OAuth Consent Screen (Google) +![Google Consent](./screenshots/google-consent.png) +*User experience during Google OAuth flow* + +## Breaking Changes +None - this is additive functionality that doesn't affect existing authentication methods. + +## Security Considerations +- OAuth tokens are stored securely using bcrypt hashing +- CSRF protection implemented for all OAuth routes +- State parameter used to prevent CSRF attacks +- No secrets committed to repository (all in .env) +- Rate limiting applied to OAuth endpoints + +## Additional Notes + +### Design Decisions +- **Chose passport.js** over custom implementation for security, maintenance, and community support +- **Used strategy pattern** to make adding new OAuth providers easier in future +- **Account merging** happens automatically based on email address (primary key) +- **No email verification required** for OAuth signups (providers already verify emails) + +### Future Improvements +Consider in follow-up PRs: +- Add more providers (Twitter, LinkedIn, Microsoft) +- Implement OAuth token refresh logic +- Add rate limiting for OAuth endpoints +- Add admin dashboard for managing OAuth apps + +### Migration Notes +No migration needed - existing users can continue using password authentication. OAuth is an additional option. + +### Dependencies Added +- passport v0.6.0 +- passport-google-oauth20 v2.0.0 +- passport-github2 v0.1.12 + +All dependencies are actively maintained and have good security track records. +``` + +--- + +### Files Changed (Clean!) + +``` +.env.example # Environment variable examples (no secrets!) +README.md # Updated setup instructions +docs/authentication.md # OAuth documentation +package.json # Added passport dependencies +src/routes/auth.js # OAuth routes +src/middleware/authenticate.js # OAuth middleware +src/models/user.js # Updated user model +tests/auth/oauth.test.js # OAuth tests +tests/fixtures/users.json # Test fixtures +``` + +**What's NOT included:** +- ❌ No SESSION.md or notes files +- ❌ No planning/ directory +- ❌ No debug screenshots (only feature demos) +- ❌ No temporary test files +- ❌ No .env file (only .env.example) +- ❌ No personal artifacts + +--- + +### Commit History (Clean!) + +``` +feat(auth): add OAuth2 support for Google and GitHub + +Implemented OAuth2 authentication flow using passport.js. +Users can now sign in with Google or GitHub accounts. +Accounts are automatically merged if email matches existing user. + +- Added OAuth routes and callbacks +- Updated user model to support social accounts +- Added comprehensive tests for OAuth flow +- Documented setup and usage + +Closes #234 +``` + +**Why it's good:** +- ✅ Single, focused commit +- ✅ Clear commit message +- ✅ Explains what and why +- ✅ Links issue +- ✅ Follows Conventional Commits + +--- + +### Communication During Review + +**Initial Comment (When Submitting):** +``` +Hi @maintainer! 👋 + +I've implemented OAuth2 support as discussed in #234. I went with passport.js over a custom implementation because: +1. Battle-tested security +2. Well-maintained by the community +3. Easy to add more providers in future + +I've tested this locally for the past week and also deployed to staging for integration testing. All existing auth flows remain unchanged - this is purely additive. + +Ready for review when you have time! Happy to make any changes you'd like. +``` + +**Response to Feedback:** +``` +> Could you add rate limiting to these endpoints? + +Good idea! I've added rate limiting in commit abc1234: +- Max 5 OAuth attempts per IP per minute +- Returns 429 with Retry-After header +- Uses existing rate limiting middleware + +Let me know if you'd prefer different limits! +``` + +**After Changes:** +``` +@maintainer I've addressed all your feedback: +- ✅ Added rate limiting (commit abc1234) +- ✅ Updated docs with security considerations (commit def5678) +- ✅ Refactored callback logic as suggested (commit ghi9012) + +Marked all conversations as resolved. Ready for re-review! + +Thanks for the thorough feedback - the rate limiting suggestion was spot-on. +``` + +--- + +### PR Metrics + +**Size:** +- Lines changed: 180 +- Files changed: 9 +- Commits: 1 + +**Result:** +- ✅ Ideal size for review +- ✅ Focused on one feature +- ✅ Easy to understand +- ✅ Quick to review + +**Timeline:** +- Submitted: Day 1 +- First review: Day 2 +- Changes made: Day 2 +- Approved: Day 3 +- Merged: Day 3 + +**Why it was quick:** +- Clean, focused PR +- Comprehensive testing +- Good documentation +- Responsive to feedback +- No surprises or issues + +--- + +## Key Takeaways + +This PR demonstrates: + +1. **Clear Communication** + - What/Why/How structure + - Testing instructions + - Design decisions explained + - Security considerations noted + +2. **Proper Scope** + - One feature + - ~180 lines changed + - No unrelated changes + - Easy to review + +3. **Complete Documentation** + - Updated README + - Added OAuth docs + - Included .env.example + - Comprehensive inline comments + +4. **Thorough Testing** + - Automated tests + - Manual test steps + - Edge cases covered + - Deployed to staging first + +5. **Professional Artifacts** + - No personal files + - No secrets + - Clean commit history + - Proper branch naming + +6. **Excellent Communication** + - Polite and respectful + - Responsive to feedback + - Explains decisions + - Thanks reviewers + +7. **Security Conscious** + - No secrets committed + - CSRF protection + - Rate limiting + - Security considerations documented + +This is the standard to aim for in your pull requests! diff --git a/plugin.lock.json b/plugin.lock.json new file mode 100644 index 0000000..681dea6 --- /dev/null +++ b/plugin.lock.json @@ -0,0 +1,81 @@ +{ + "$schema": "internal://schemas/plugin.lock.v1.json", + "pluginId": "gh:jezweb/claude-skills:skills/open-source-contributions", + "normalized": { + "repo": null, + "ref": "refs/tags/v20251128.0", + "commit": "7ae82902e2041f78650cf6c998468d6077cb54b2", + "treeHash": "a5ed0be3adad9b9e436d1b851f17d92f37cda3323b14344a4902f39c4d4d26c2", + "generatedAt": "2025-11-28T10:19:05.922861Z", + "toolVersion": "publish_plugins.py@0.2.0" + }, + "origin": { + "remote": "git@github.com:zhongweili/42plugin-data.git", + "branch": "master", + "commit": "aa1497ed0949fd50e99e70d6324a29c5b34f9390", + "repoRoot": "/Users/zhongweili/projects/openmind/42plugin-data" + }, + "manifest": { + "name": "open-source-contributions", + "description": "Create maintainer-friendly pull requests for open source projects with clean code submissions and professional communication. Prevents 16 common mistakes that cause PR rejection. Use when: contributing to public repositories, submitting PRs to community projects, migrating from contributor to maintainer workflows, or troubleshooting PR rejection, working on main branch errors, failing CI checks, or personal artifacts in commits.", + "version": "1.0.0" + }, + "content": { + "files": [ + { + "path": "LICENSE", + "sha256": "b76f0805c81dad30acd0fa7aca893e4f834986e168549201c42a92e810956072" + }, + { + "path": "README.md", + "sha256": "bc84341ca3867a7d42dd9d89d8eaa0326b6f9a76de91fb6a31d50895f53885b1" + }, + { + "path": "SKILL.md", + "sha256": "1d6898ec2795b969f4768a253e4ac5780193f69ac34a291ae479531276698ede" + }, + { + "path": "references/commit-message-guide.md", + "sha256": "99de91dcfbc148f82c1c31bd2d115a13863a113e287249abe4c1de93cddf0fea" + }, + { + "path": "references/files-to-exclude.md", + "sha256": "296fd9310d53d57c48654e1e13876b8fe0e2c5d11da8d5bf51cc8f923e8bc82a" + }, + { + "path": "references/pr-template.md", + "sha256": "cb077517e050d9610bf6fe83a7bb00767513be46d218f223d3beaee5a7b7b4a4" + }, + { + "path": "references/pr-checklist.md", + "sha256": "69ba9e481e6e6209fc4d17fea8588886eb0634f8978daaa76fc3b59b3c542b42" + }, + { + "path": "scripts/pre-pr-check.sh", + "sha256": "6e21d0c55651921b270f166f7cbbea244f0578506d78f984663c0c83497b4587" + }, + { + "path": "scripts/clean-branch.sh", + "sha256": "0183418de9c4414aa40006a7a26f9c6ee04cade92de5eefe1bc5b5d1110e372c" + }, + { + "path": ".claude-plugin/plugin.json", + "sha256": "0a0aa15f64627b970f0ca3b0bbf86390474a0994d963971ff72c438714d61434" + }, + { + "path": "assets/good-pr-example.md", + "sha256": "11cf4e7376469c5bc334b54936fb794ecaffe5b09d9deaa857b832fb54a51d5c" + }, + { + "path": "assets/bad-pr-example.md", + "sha256": "29cdd1bc4f676062dacf397342b366c08eb61f8637fb06ac08641436b4be3ab0" + } + ], + "dirSha256": "a5ed0be3adad9b9e436d1b851f17d92f37cda3323b14344a4902f39c4d4d26c2" + }, + "security": { + "scannedAt": null, + "scannerVersion": null, + "flags": [] + } +} \ No newline at end of file diff --git a/references/commit-message-guide.md b/references/commit-message-guide.md new file mode 100644 index 0000000..ca85607 --- /dev/null +++ b/references/commit-message-guide.md @@ -0,0 +1,534 @@ +# Commit Message Guide + +A comprehensive guide to writing clear, consistent, and maintainable commit messages using the Conventional Commits format. + +--- + +## Why Good Commit Messages Matter + +- **Documentation**: Git history becomes a changelog +- **Code Review**: Easier to understand what changed and why +- **Debugging**: Quickly find when bugs were introduced +- **Automation**: Enables automatic changelog generation and versioning +- **Professionalism**: Shows attention to detail and respect for maintainers + +--- + +## Conventional Commits Format + +``` +(): + +[optional body] + +[optional footer(s)] +``` + +### Components + +**Type** (required): Category of change +**Scope** (optional): What section of codebase is affected +**Subject** (required): Brief description of change +**Body** (optional): Detailed explanation +**Footer** (optional): Issue references, breaking changes + +--- + +## Types + +### Primary Types + +**feat**: New feature for the user +``` +feat(auth): add OAuth2 support for Google and GitHub +``` + +**fix**: Bug fix for the user +``` +fix(api): resolve memory leak in worker shutdown +``` + +**docs**: Documentation changes only +``` +docs(readme): update installation instructions for v2.0 +``` + +**style**: Code style changes (formatting, missing semicolons, etc.) +``` +style(components): format code with Prettier +``` + +**refactor**: Code change that neither fixes bug nor adds feature +``` +refactor(auth): extract middleware to separate module +``` + +**perf**: Performance improvement +``` +perf(database): add index to user_id column +``` + +**test**: Adding or updating tests +``` +test(auth): add OAuth flow integration tests +``` + +### Supporting Types + +**build**: Changes to build system or dependencies +``` +build(deps): upgrade React to v18.2.0 +``` + +**ci**: CI configuration changes +``` +ci(github): add automated release workflow +``` + +**chore**: Other changes that don't modify src or test files +``` +chore(deps): update dev dependencies +``` + +**revert**: Revert a previous commit +``` +revert: feat(auth): add OAuth2 support + +This reverts commit abc123def456. +``` + +--- + +## Subject Line Rules + +### The 7 Rules + +1. **Limit to 50 characters** (hard limit: 72) +2. **Use imperative mood** ("Add" not "Added" or "Adds") +3. **Capitalize first word** after the colon +4. **Don't end with punctuation** +5. **Focus on WHAT changed** (body explains WHY) +6. **Be specific** but concise +7. **Start with type** (if using Conventional Commits) + +### Examples + +✅ **Good:** +``` +feat(api): add user authentication endpoint +fix(ui): resolve button alignment on mobile +docs(contributing): clarify PR submission process +refactor(utils): simplify date formatting logic +``` + +❌ **Bad:** +``` +Fixed stuff # Too vague +Added new feature to the authentication system that allows users to login with OAuth # Too long +updated code # Not specific, wrong tense +feat(api): add user authentication. # Don't end with period +Feat(api): add auth # Don't capitalize type +``` + +--- + +## Optional Scope + +The scope provides context about what part of the codebase changed. + +### Common Scopes + +``` +feat(auth): ... # Authentication +feat(api): ... # API layer +feat(ui): ... # User interface +feat(database): ... # Database +feat(docs): ... # Documentation +feat(tests): ... # Tests +fix(deps): ... # Dependencies +build(webpack): ... # Build tooling +``` + +### Project-Specific Scopes + +Check the project's recent commits for conventions: +```bash +git log --oneline -20 +``` + +### When to Omit Scope + +Scope is optional. Omit when: +- Change affects multiple areas +- Project doesn't use scopes +- Scope would be too generic + +``` +feat: add dark mode support +docs: update README +``` + +--- + +## Body (Optional) + +The body provides detailed explanation of WHAT and WHY (code shows HOW). + +### Rules + +- Separate from subject with blank line +- Wrap at 72 characters per line +- Explain motivation and context +- Contrast with previous behavior +- Use bullet points for multiple points + +### Example + +``` +feat(api): add rate limiting to authentication endpoints + +Users can now make up to 100 authentication attempts per hour +per IP address. This prevents brute force attacks while allowing +legitimate users to retry failed login attempts. + +Implementation details: +- Uses Redis for distributed rate limiting +- Configurable via RATE_LIMIT_MAX environment variable +- Returns 429 status with Retry-After header when exceeded +- Resets hourly at top of the hour + +Previous behavior allowed unlimited attempts, which was identified +as a security vulnerability in audit #456. +``` + +### When to Include Body + +Include a body when: +- Change is complex or non-obvious +- Design decisions need explanation +- Previous behavior needs context +- Multiple related changes in one commit +- Security or performance implications + +### When to Skip Body + +Skip the body when: +- Change is self-explanatory +- Subject line tells the complete story +- Trivial changes (typos, formatting) + +--- + +## Footer (Optional) + +Footers provide metadata about the commit. + +### Breaking Changes + +Use `BREAKING CHANGE:` footer for breaking changes: + +``` +feat(api)!: change authentication endpoint path + +BREAKING CHANGE: The authentication endpoint has moved from +/api/auth to /api/v2/auth. Update your API calls accordingly. + +Migration guide available at docs/migration/v2.md +``` + +Note the `!` after the type indicates a breaking change. + +### Issue References + +Link issues using keywords for automatic closing: + +``` +fix(ui): resolve mobile layout issues + +Fixes #123 +Closes #456, #789 +Relates to #234 +``` + +**Closing Keywords:** +- `Closes #123` +- `Fixes #123` +- `Resolves #123` +- Also: close, fix, resolve (case insensitive) + +### Co-authored-by + +Credit co-authors: + +``` +feat(api): add GraphQL support + +Co-authored-by: Jane Doe +Co-authored-by: John Smith +``` + +### Other Footers + +``` +Reviewed-by: Name +Signed-off-by: Name +Acked-by: Name +See-also: #123 +``` + +--- + +## Complete Examples + +### Example 1: Simple Feature + +``` +feat(ui): add dark mode toggle + +Adds a toggle button in settings to switch between light and dark +themes. User preference is saved to localStorage and persists +across sessions. + +Closes #234 +``` + +### Example 2: Bug Fix + +``` +fix(api): prevent race condition in cache invalidation + +The cache invalidation logic wasn't thread-safe, causing occasional +race conditions when multiple workers tried to invalidate the same +key simultaneously. + +Solution: +- Added mutex locks around the critical section +- Implemented timeout for lock acquisition (5s) +- Added retry logic with exponential backoff +- Updated tests to verify thread-safety + +Fixes #456 +``` + +### Example 3: Breaking Change + +``` +feat(api)!: migrate to TypeScript and update endpoint contracts + +BREAKING CHANGE: All API endpoints now return ISO 8601 date +strings instead of Unix timestamps. Update client code to parse +dates accordingly. + +Additionally, authentication now requires JWT tokens in the +Authorization header instead of session cookies. + +Migration guide: docs/migration/v3.md + +Closes #567 +``` + +### Example 4: Refactoring + +``` +refactor(auth): extract middleware to separate module + +No functional changes, but auth logic is now easier to test and +maintain. Consolidated duplicate code from 5 route handlers into +reusable middleware functions. + +Files affected: +- New: middleware/authenticate.js +- Updated: routes/*.js (5 files) +- Tests: tests/middleware/auth.test.js + +Relates to #301 (technical debt epic) +``` + +### Example 5: Documentation + +``` +docs(api): add examples for authentication flows + +Added code examples for: +- Basic authentication with username/password +- OAuth2 flow with Google +- API key authentication +- JWT token refresh + +Examples include curl commands and JavaScript fetch() snippets. + +Closes #678 +``` + +### Example 6: Multiple Related Changes + +``` +fix(auth): resolve multiple OAuth edge cases + +- Handle expired refresh tokens gracefully +- Prevent account linking when email doesn't match +- Add rate limiting to token refresh endpoint +- Log failed OAuth attempts for security monitoring + +Each issue was related to OAuth implementation and fixing them +separately would have caused merge conflicts. + +Fixes #123, #456, #789 +``` + +--- + +## Tips for Writing Good Commit Messages + +### Do: + +✅ Write in imperative mood ("Add" not "Added") +``` +feat: add user profile page +``` + +✅ Be specific about what changed +``` +fix(api): resolve timeout in user search endpoint +``` + +✅ Explain WHY in the body +``` +refactor(db): switch to connection pooling + +The previous approach created a new connection for each request, +which caused performance issues under load. Connection pooling +reduces overhead and improves response times by 40%. +``` + +✅ Use the body for complex changes +✅ Reference issues and PRs +✅ Follow project conventions + +### Don't: + +❌ Use past tense +``` +feat: added user profile page ❌ +``` + +❌ Be vague +``` +fix: bug fix ❌ +update: changes ❌ +``` + +❌ Write novels in the subject line +``` +feat(api): add new user authentication endpoint with OAuth2 support for Google and GitHub that also includes rate limiting ❌ +``` + +❌ Skip the type (if project uses Conventional Commits) +``` +Add user profile page ❌ +``` + +❌ Use abbreviations or jargon unnecessarily +``` +fix(db): rm dup recs via opt idx ❌ +``` + +❌ Combine unrelated changes in one commit +``` +feat: add dark mode, fix typo in README, update dependencies ❌ +``` + +--- + +## Real-World Examples from Popular Projects + +### React +``` +feat(react-dom): Add support for CSS Layers + +Implements support for @layer, enabling better CSS encapsulation. + +Fixes #24556 +``` + +### Node.js +``` +doc: add missing types to request and response + +Added TypeScript type definitions for several Request and Response +methods that were previously missing from the declarations. + +Fixes: #12345 +Refs: #67890 +``` + +### Kubernetes +``` +fix: prevent pod creation with invalid security context + +Adds validation to reject pods with both `runAsUser: 0` and +`allowPrivilegeEscalation: false`, which is an invalid combination. + +Closes #12345 +``` + +--- + +## Commit Message Checklist + +Before committing, verify: + +- [ ] Type is correct (feat, fix, docs, etc.) +- [ ] Subject line under 50 characters (max 72) +- [ ] Imperative mood ("Add" not "Added") +- [ ] First word capitalized +- [ ] No period at end +- [ ] Body explains WHY (if needed) +- [ ] Body wrapped at 72 characters +- [ ] Blank line between subject and body +- [ ] Issues referenced in footer +- [ ] Breaking changes noted with BREAKING CHANGE: +- [ ] Follows project conventions + +--- + +## Tools & Automation + +### Commitizen + +Interactive tool for writing commits: +```bash +npm install -g commitizen +git cz +``` + +### Commitlint + +Lint commit messages: +```bash +npm install --save-dev @commitlint/cli @commitlint/config-conventional +``` + +### Husky + +Git hooks to enforce commit message format: +```bash +npm install --save-dev husky +npx husky add .husky/commit-msg 'npx --no -- commitlint --edit ${1}' +``` + +--- + +## Resources + +**Specifications:** +- Conventional Commits: https://www.conventionalcommits.org/ +- Git Commit Guidelines: https://git-scm.com/book/en/v2/Distributed-Git-Contributing-to-a-Project + +**Tools:** +- Commitizen: https://github.com/commitizen/cz-cli +- Commitlint: https://commitlint.js.org/ + +**Further Reading:** +- "How to Write a Git Commit Message": https://chris.beams.io/posts/git-commit/ +- Angular Commit Guidelines: https://github.com/angular/angular/blob/main/CONTRIBUTING.md#commit diff --git a/references/files-to-exclude.md b/references/files-to-exclude.md new file mode 100644 index 0000000..921f960 --- /dev/null +++ b/references/files-to-exclude.md @@ -0,0 +1,533 @@ +# Files to Exclude from Pull Requests + +A comprehensive reference of files that should NEVER be included in open source pull requests, organized by category. + +--- + +## Personal Development Artifacts + +### Session & Notes Files +``` +❌ SESSION.md # Session tracking +❌ NOTES.md # Development notes +❌ TODO.md # Personal todo lists +❌ SCRATCH.md # Scratch notes +❌ DEBUGGING.md # Debugging notes +❌ TESTING.md # Testing notes +❌ JOURNAL.md # Development journal +❌ IDEAS.md # Personal ideas +❌ THOUGHTS.md # Random thoughts +❌ WIP.md # Work in progress notes +``` + +### Planning Documents +``` +❌ planning/ # Entire planning directory +❌ IMPLEMENTATION_PHASES.md # Phase-based planning +❌ DATABASE_SCHEMA.md # Database planning (unless adding to project) +❌ ARCHITECTURE.md # Architecture planning (unless adding to project) +❌ API_ENDPOINTS.md # API planning (unless adding to project) +❌ UI_COMPONENTS.md # UI planning (unless adding to project) +❌ PROJECT_SPEC.md # Project specifications +❌ ROADMAP.md # Personal roadmap +❌ MILESTONES.md # Personal milestones +``` + +### Research & Reference +``` +❌ research-logs/ # Research directory +❌ references/ # Personal references (skill-specific) +❌ research-*.md # Research documents +❌ analysis-*.md # Analysis documents +❌ comparison-*.md # Comparison documents +❌ evaluation-*.md # Evaluation documents +``` + +--- + +## Screenshots & Visual Assets + +### Debug & Development Screenshots +``` +❌ screenshots/debug-*.png # Debugging screenshots +❌ screenshots/test-*.png # Testing screenshots +❌ screenshots/scratch-*.png # Scratch screenshots +❌ screenshot-*.png # Ad-hoc screenshots +❌ screen-recording-*.mp4 # Screen recordings +❌ before-after-local.png # Local comparisons +❌ demo-local.* # Local demos +❌ temp-visual.* # Temporary visuals +``` + +### When Screenshots ARE Okay +``` +✅ docs/assets/ui-example.png # Documentation assets +✅ screenshots/feature-demo.png # Demonstrating feature in PR description +✅ docs/images/architecture.png # Architecture diagrams for project docs +``` + +**Rule of Thumb**: Only include screenshots if they: +1. Demonstrate a feature for the PR description +2. Are part of documentation updates +3. Would be useful to all users/developers + +--- + +## Test Files (Situational) + +### Temporary Test Files (NEVER Include) +``` +❌ test-manual.js # Manual testing scripts +❌ test-debug.ts # Debugging tests +❌ test-quick.py # Quick validation tests +❌ scratch-test.sh # Scratch test scripts +❌ example-local.json # Local test data +❌ quick-test.* # Quick test files +❌ debug-test.* # Debug test files +❌ temp-test.* # Temporary tests +❌ playground.* # Playground files +❌ experiment.* # Experimental files +``` + +### Proper Test Files (Include These) +``` +✅ tests/feature.test.js # Proper test suite +✅ tests/fixtures/data.json # Required test fixtures +✅ __tests__/component.tsx # Component tests +✅ spec/feature_spec.rb # RSpec tests +✅ test_feature.py # Python tests +``` + +**Rule**: Only include tests that: +1. Are part of the project's test suite structure +2. Follow project's testing conventions +3. Will be run by CI/other developers +4. Test the specific feature/fix in the PR + +--- + +## Build Artifacts & Dependencies + +### Build Output +``` +❌ dist/ # Build output +❌ build/ # Build directory +❌ out/ # Output directory +❌ target/ # Rust/Java build directory +❌ bin/ # Binary output (usually) +❌ lib/ # Library output (usually) +❌ *.exe, *.dll, *.so # Compiled binaries +❌ *.o, *.obj # Object files +❌ *.pyc, *.pyo # Python compiled files +❌ __pycache__/ # Python cache +❌ .next/ # Next.js build +❌ .nuxt/ # Nuxt build +❌ .output/ # Nitro build +``` + +### Dependencies +``` +❌ node_modules/ # Node dependencies +❌ vendor/ # Ruby/PHP dependencies +❌ venv/ # Python virtual environment +❌ .venv/ # Python virtual environment +❌ env/ # Environment directory +❌ Cargo.lock # Rust dependencies (situational) +❌ package-lock.json # NPM lock (situational) +❌ yarn.lock # Yarn lock (situational) +❌ pnpm-lock.yaml # PNPM lock (situational) +``` + +**Lock File Rule**: Only include lock files if: +- Project explicitly requires them (check CONTRIBUTING.md) +- You're adding/updating dependencies +- Project uses lock files (check existing files in repo) + +### Cache & Temporary Build Files +``` +❌ .cache/ # Cache directory +❌ .tmp/ # Temporary files +❌ .temp/ # Temporary files +❌ tmp/ # Temporary directory +❌ temp/ # Temporary directory +❌ *.cache # Cache files +❌ .eslintcache # ESLint cache +❌ .parcel-cache/ # Parcel cache +❌ .turbo/ # Turborepo cache +``` + +--- + +## IDE & Editor Files + +### VS Code +``` +❌ .vscode/ # VS Code settings (use global gitignore) +❌ *.code-workspace # Workspace files +``` + +### JetBrains (IntelliJ, WebStorm, etc.) +``` +❌ .idea/ # IntelliJ settings +❌ *.iml # Module files +❌ *.ipr # Project files +❌ *.iws # Workspace files +``` + +### Vim +``` +❌ *.swp # Swap files +❌ *.swo # Swap files +❌ *~ # Backup files +❌ .*.sw? # Swap files pattern +``` + +### Emacs +``` +❌ *~ # Backup files +❌ \#*\# # Auto-save files +❌ .\#* # Lock files +``` + +### Sublime Text +``` +❌ *.sublime-project # Project files +❌ *.sublime-workspace # Workspace files +``` + +**Exception**: If the project provides official IDE configurations (like .vscode/settings.json in the repo), you may update those if needed for the feature. + +--- + +## Operating System Files + +### macOS +``` +❌ .DS_Store # Finder metadata +❌ .AppleDouble # Resource forks +❌ .LSOverride # Icon metadata +❌ ._* # Resource forks +``` + +### Windows +``` +❌ Thumbs.db # Thumbnail cache +❌ ehthumbs.db # Thumbnail cache +❌ Desktop.ini # Folder settings +❌ $RECYCLE.BIN/ # Recycle bin +``` + +### Linux +``` +❌ .directory # KDE directory settings +❌ .Trash-*/ # Trash directory +``` + +--- + +## Secrets & Credentials (CRITICAL!) + +### Environment Files +``` +❌ .env # Environment variables (NEVER!) +❌ .env.local # Local environment +❌ .env.development # Development environment +❌ .env.production # Production environment (NEVER!) +❌ .env.*.local # Any local env files +❌ config/local.json # Local configuration +❌ config/secrets.json # Secrets configuration +``` + +### Credentials +``` +❌ credentials.json # Credentials file +❌ secrets.json # Secrets file +❌ auth.json # Authentication file +❌ token.txt # Token files +❌ api-keys.json # API keys +❌ service-account.json # Service account credentials +``` + +### Keys & Certificates +``` +❌ *.key # Private keys +❌ *.pem # PEM certificates +❌ *.p12 # PKCS#12 certificates +❌ *.pfx # PFX certificates +❌ id_rsa # SSH private key +❌ id_dsa # SSH private key +❌ *.crt (sometimes) # Certificates +``` + +### Password & Secret Patterns +Look for these in file contents: +``` +❌ password= +❌ api_key= +❌ api-key= +❌ apiKey= +❌ secret= +❌ token= +❌ access_token= +❌ private_key= +``` + +**CRITICAL**: Even if deleted later, secrets in git history are compromised. Use `git filter-branch` or BFG Repo-Cleaner if secrets are committed. + +--- + +## Logs & Debugging + +### Log Files +``` +❌ *.log # Log files +❌ logs/ # Logs directory +❌ debug.log # Debug logs +❌ error.log # Error logs +❌ npm-debug.log # NPM debug logs +❌ yarn-debug.log # Yarn debug logs +❌ yarn-error.log # Yarn error logs +❌ lerna-debug.log # Lerna debug logs +``` + +### Debug Files +``` +❌ debug-*.js # Debug scripts +❌ debug-*.py # Debug scripts +❌ trace-*.txt # Trace files +❌ profile-*.json # Profiling output +❌ *.prof # Profiling files +❌ *.trace # Trace files +``` + +### Crash Dumps +``` +❌ core # Core dumps +❌ core.* # Core dumps +❌ *.dmp # Dump files +❌ crash-*.log # Crash logs +``` + +--- + +## Database & Data Files + +### Database Files (Local Development) +``` +❌ *.db # SQLite databases (local) +❌ *.sqlite # SQLite databases (local) +❌ *.sqlite3 # SQLite databases (local) +❌ dump.sql # Database dumps +❌ backup.sql # Database backups +❌ *.mdb # Access databases +``` + +### Data Files (Local/Personal) +``` +❌ data/local/ # Local data directory +❌ data/personal/ # Personal data +❌ data/test-data.json # Test data (unless fixtures) +❌ sample-data-local.json # Local sample data +``` + +**Exception**: Include database files if: +- They're part of the project's test fixtures +- They're example/seed data for the project +- Project explicitly includes them (check existing repo) + +--- + +## Coverage & Reports + +### Test Coverage +``` +❌ coverage/ # Coverage reports +❌ .coverage # Coverage data +❌ htmlcov/ # HTML coverage +❌ .nyc_output/ # NYC coverage +❌ lcov.info # LCOV coverage +``` + +### Reports +``` +❌ reports/ # Generated reports +❌ test-results/ # Test results +❌ junit.xml # JUnit reports +❌ cypress/videos/ # Cypress videos +❌ cypress/screenshots/ # Cypress screenshots (unless demonstrating bug) +``` + +--- + +## Version Control (Other Than Git) + +### SVN +``` +❌ .svn/ # SVN metadata +``` + +### Mercurial +``` +❌ .hg/ # Mercurial metadata +❌ .hgignore # Mercurial ignore +``` + +### CVS +``` +❌ CVS/ # CVS metadata +❌ .cvsignore # CVS ignore +``` + +--- + +## What SHOULD Be Included + +For reference, these ARE okay to include: + +### Source Code +``` +✅ src/ # Source code +✅ lib/ # Library code (if source, not compiled) +✅ app/ # Application code +✅ components/ # Component files +✅ utils/ # Utility functions +``` + +### Tests +``` +✅ tests/ # Test directory +✅ __tests__/ # Jest tests +✅ spec/ # RSpec tests +✅ test_*.py # Python tests +``` + +### Documentation +``` +✅ README.md # Project readme +✅ CHANGELOG.md # Changelog +✅ CONTRIBUTING.md # Contributing guide +✅ LICENSE # License file +✅ docs/ # Documentation directory +``` + +### Configuration (Project-level) +``` +✅ .gitignore # Git ignore rules +✅ .eslintrc # ESLint config (if updating) +✅ .prettierrc # Prettier config (if updating) +✅ tsconfig.json # TypeScript config (if updating) +✅ package.json # NPM package file (if updating) +✅ Cargo.toml # Rust config (if updating) +✅ pyproject.toml # Python config (if updating) +``` + +### CI/CD (if part of feature) +``` +✅ .github/workflows/ # GitHub Actions +✅ .gitlab-ci.yml # GitLab CI +✅ .travis.yml # Travis CI +✅ Jenkinsfile # Jenkins +``` + +### Migrations & Schema (if part of feature) +``` +✅ migrations/ # Database migrations +✅ schema.sql # Database schema (if adding to project) +✅ seeds/ # Seed data (if part of project) +``` + +--- + +## How to Prevent Including These Files + +### 1. Project .gitignore +Add patterns that benefit ALL developers: +```gitignore +# Build +dist/ +build/ +*.pyc + +# Dependencies +node_modules/ +vendor/ + +# Logs +*.log + +# Secrets +.env +.env.local +*.key +*.pem +``` + +### 2. Global .gitignore (Recommended) +Add personal/OS-specific patterns: +```bash +# Configure global gitignore +git config --global core.excludesfile ~/.gitignore_global + +# Add your patterns +echo ".DS_Store" >> ~/.gitignore_global +echo ".vscode/" >> ~/.gitignore_global +echo ".idea/" >> ~/.gitignore_global +echo "*.swp" >> ~/.gitignore_global +``` + +### 3. Local Exclusions (.git/info/exclude) +For patterns specific to YOUR workflow only: +```bash +echo "SESSION.md" >> .git/info/exclude +echo "NOTES.md" >> .git/info/exclude +echo "planning/" >> .git/info/exclude +echo "screenshots/debug-*" >> .git/info/exclude +``` + +**Difference**: +- `.gitignore` → Committed, affects everyone +- `~/.gitignore_global` → Your global settings, affects all your repos +- `.git/info/exclude` → This repo only, not committed + +--- + +## Quick Check Commands + +### List all tracked files: +```bash +git ls-files +``` + +### Check for specific patterns: +```bash +git ls-files | grep -E "SESSION|NOTES|TODO|planning" +``` + +### Find large files: +```bash +git ls-files | while read file; do + [ -f "$file" ] && stat -f%z "$file" "$file" +done | sort -rn | head -20 +``` + +### Search for secrets in staged files: +```bash +git diff --cached | grep -iE "password|secret|api[_-]?key|token" +``` + +### Use the pre-PR check script: +```bash +./scripts/pre-pr-check.sh +``` + +--- + +## Resources + +- **Pre-PR Check Script**: Automated scanning for these patterns +- **Clean Branch Script**: Remove common artifacts safely +- **GitHub's gitignore templates**: https://github.com/github/gitignore + +--- + +**Remember**: When in doubt, DON'T include it. You can always add files later if needed, but removing them from git history is much harder. diff --git a/references/pr-checklist.md b/references/pr-checklist.md new file mode 100644 index 0000000..f4d8fca --- /dev/null +++ b/references/pr-checklist.md @@ -0,0 +1,351 @@ +# Pull Request Submission Checklist + +A comprehensive checklist to ensure your PR is ready for review and meets open source contribution standards. + +--- + +## Pre-Contribution Phase + +**Before starting any work:** + +- [ ] **Read CONTRIBUTING.md** - Found in root, .github/, or docs/ +- [ ] **Read CODE_OF_CONDUCT.md** - Understand community expectations +- [ ] **Check if issue exists** - Search existing issues for your topic +- [ ] **Create issue first (if needed)** - For significant changes, discuss before coding +- [ ] **Comment on issue to claim work** - Prevents duplicate effort + ``` + "Hi! I'd like to work on this. My approach would be to [brief outline]." + ``` +- [ ] **Wait for acknowledgment** - Especially for large changes +- [ ] **Fork the repository** - If you don't have write access +- [ ] **Set up upstream remote** + ```bash + git remote add upstream https://github.com/ORIGINAL/repo.git + ``` +- [ ] **Create feature branch** - NEVER work on main/master + ```bash + git checkout -b feature/descriptive-name + ``` +- [ ] **Understand testing requirements** - Check what tests are expected +- [ ] **Identify code style tools** - Look for .eslintrc, .prettierrc, etc. + +--- + +## Development Phase + +**While coding:** + +- [ ] **Follow project style guidelines** - Match existing code patterns +- [ ] **Write tests for new functionality** - Most projects require this +- [ ] **Update tests for changed behavior** - Keep tests in sync with code +- [ ] **Add/update documentation** - README, API docs, inline comments +- [ ] **Keep commits atomic** - One logical change per commit +- [ ] **Write good commit messages** - Follow Conventional Commits format + ``` + feat(scope): brief description + + Longer explanation if needed + + Fixes #123 + ``` +- [ ] **Run linters and formatters** - Fix style issues during development + ```bash + npm run lint + npm run format + ``` +- [ ] **Test locally frequently** - Don't wait until the end +- [ ] **Keep PR scope focused** - One feature/fix per PR +- [ ] **Sync with upstream regularly** - Avoid merge conflicts + ```bash + git fetch upstream + git rebase upstream/main + ``` + +--- + +## Pre-Submission Phase + +### Code Quality + +- [ ] **All tests pass locally** + ```bash + npm test + # or + pytest + # or + cargo test + # or project's test command + ``` +- [ ] **Code builds successfully** + ```bash + npm run build + ``` +- [ ] **Linter passes** + ```bash + npm run lint + ``` +- [ ] **Formatter applied** + ```bash + npm run format + ``` +- [ ] **Code coverage maintained** - If project has minimum thresholds +- [ ] **No compiler warnings** - Clean build output +- [ ] **Manual testing completed** - Test the actual functionality + +### Code Review (Self) + +- [ ] **Review your own diff** + ```bash + git diff origin/main + ``` +- [ ] **No debugging code** - Remove console.logs, debugger statements +- [ ] **No commented-out code** - Remove dead code +- [ ] **No TODO comments** - Complete work or create follow-up issues +- [ ] **Consistent naming** - Variables, functions, files match conventions +- [ ] **Proper error handling** - Don't swallow errors silently +- [ ] **Edge cases considered** - Null checks, empty arrays, etc. + +### Cleanup (Critical!) + +Run the pre-PR check script: +```bash +./scripts/pre-pr-check.sh +``` + +- [ ] **Remove SESSION.md** - Personal session tracking +- [ ] **Remove NOTES.md** - Development notes +- [ ] **Remove TODO.md** - Personal todo lists +- [ ] **Remove planning/* directory** - Project planning documents +- [ ] **Remove debug screenshots** - Screenshots used during debugging + - Keep only screenshots demonstrating features for PR description +- [ ] **Remove temporary test files** + - test-manual.js, test-debug.ts, quick-test.py +- [ ] **Remove personal workflow files** + - scratch.*, temp.*, debug.* +- [ ] **No IDE/editor files** + - .vscode/, .idea/, *.swp + - Should be in global .gitignore, not committed +- [ ] **No OS-specific files** + - .DS_Store, Thumbs.db, desktop.ini +- [ ] **No build artifacts** + - dist/, build/, node_modules/, __pycache__/ +- [ ] **No secrets or credentials** + - .env, credentials.json, *.key, *.pem + - Double-check with: `git diff | grep -i "password\|secret\|key"` +- [ ] **No large binary files** - Unless necessary for the PR +- [ ] **No unrelated changes** - Only changes relevant to this PR + +### Git Hygiene + +- [ ] **Commits are clean** - No "WIP" or "fix typo" commits + - Consider squashing if needed +- [ ] **Commit messages follow conventions** - Conventional Commits format +- [ ] **No merge conflicts** - Rebase on latest upstream/main + ```bash + git fetch upstream + git rebase upstream/main + ``` +- [ ] **Branch is up to date** - Latest changes from main included +- [ ] **On feature branch** - Not on main/master + +--- + +## PR Creation Phase + +### PR Description + +- [ ] **Title follows conventions** - Conventional Commits format + ``` + feat(auth): add OAuth2 support + fix(api): resolve memory leak + docs(readme): update installation + ``` +- [ ] **Uses What/Why/How structure** + - What: Brief description of changes + - Why: Reasoning and context + - How: Implementation approach +- [ ] **Testing instructions included** - Step-by-step how to test +- [ ] **Screenshots for visual changes** - Before/after if applicable +- [ ] **Breaking changes noted** - If any +- [ ] **Links to related issues** - Use closing keywords + ``` + Closes #123 + Fixes #456 + Relates to #789 + ``` +- [ ] **Checklist included** - Tests, docs, CI status +- [ ] **Description is clear** - Reviewer can understand without asking questions + +### PR Quality + +- [ ] **PR is reasonably sized** + - Ideal: < 50 lines + - Good: < 200 lines + - Max: < 400 lines + - If larger, explain why or split into multiple PRs +- [ ] **PR is focused** - One feature/fix/refactor, not multiple unrelated changes +- [ ] **No unrelated changes** - No "drive-by fixes" in unrelated files +- [ ] **All changed files are intentional** - Review git status + +### GitHub Settings + +- [ ] **Pushed to feature branch on fork** + ```bash + git push origin feature/my-feature + ``` +- [ ] **PR targets correct branch** - Usually main or develop +- [ ] **Assigned labels** - If you have permission +- [ ] **Requested reviewers** - If known and appropriate +- [ ] **Linked to project/milestone** - If applicable +- [ ] **Set as draft** - If not ready for full review yet + ```bash + gh pr create --draft + ``` + +--- + +## Post-Submission Phase + +### Monitor CI/Checks + +- [ ] **CI is running** - Green checkmarks appearing +- [ ] **All checks pass** - No failures +- [ ] **Fix failures immediately** - Don't wait for review if CI fails +- [ ] **Watch for build notifications** - Email/GitHub notifications + +### Communication + +- [ ] **Responsive to feedback** - Reply within 24-48 hours +- [ ] **Address all review comments** - Even if just "Acknowledged" +- [ ] **Mark conversations resolved** - After addressing feedback +- [ ] **Request re-review** - After making changes + ```bash + gh pr ready # if was draft + ``` +- [ ] **Thank reviewers** - Shows appreciation for their time +- [ ] **Professional tone** - Courteous and respectful +- [ ] **Ask for clarification** - If feedback is unclear +- [ ] **Be patient** - Reviewers are often volunteers + +### Updates + +- [ ] **Keep PR updated** - Rebase if main moves forward + ```bash + git fetch upstream + git rebase upstream/main + git push origin feature/my-feature --force-with-lease + ``` +- [ ] **Fix requested changes** - Implement feedback +- [ ] **Update documentation** - If requirements change +- [ ] **Squash commits** - If maintainer requests + +--- + +## Ready to Submit? + +**Final verification:** + +```bash +# 1. Run pre-PR check +./scripts/pre-pr-check.sh + +# 2. Review changes +git status +git diff origin/main --stat + +# 3. Verify tests pass +npm test + +# 4. Verify build succeeds +npm run build + +# 5. Check commit messages +git log --oneline -5 + +# 6. Push to your fork +git push origin feature/my-feature + +# 7. Create PR +gh pr create --fill +# or +gh pr create --title "feat: ..." --body "$(cat pr-description.md)" +``` + +--- + +## Common Mistakes Checklist + +Avoid these common errors: + +- [ ] ❌ Not reading CONTRIBUTING.md +- [ ] ❌ Including personal artifacts (SESSION.md, planning/*) +- [ ] ❌ Submitting massive PR (>400 lines) +- [ ] ❌ Not testing before submission +- [ ] ❌ Working on already assigned issue +- [ ] ❌ Not discussing large changes first +- [ ] ❌ Being impatient or unresponsive +- [ ] ❌ Not updating documentation +- [ ] ❌ Ignoring code style +- [ ] ❌ Ignoring CI failures +- [ ] ❌ Including unrelated changes +- [ ] ❌ Not linking issues properly +- [ ] ❌ Committing secrets +- [ ] ❌ Force-pushing without warning +- [ ] ❌ Working on main/master branch + +--- + +## Project-Specific Checklist + +Add project-specific items here based on CONTRIBUTING.md: + +- [ ] _[Project-specific requirement 1]_ +- [ ] _[Project-specific requirement 2]_ +- [ ] _[Project-specific requirement 3]_ + +--- + +## Quick Reference: Essential Commands + +```bash +# Setup +git clone https://github.com/YOUR-USERNAME/repo.git +git remote add upstream https://github.com/ORIGINAL/repo.git +git checkout -b feature/my-feature + +# Development +npm run lint +npm test +npm run build +git add . +git commit -m "feat(scope): description" + +# Pre-submission +./scripts/pre-pr-check.sh +git status +git diff origin/main + +# Submission +git push origin feature/my-feature +gh pr create --fill + +# After feedback +git fetch upstream +git rebase upstream/main +# make changes +git push origin feature/my-feature --force-with-lease +``` + +--- + +## Resources + +- **Pre-PR Check Script**: `./scripts/pre-pr-check.sh` +- **Clean Branch Script**: `./scripts/clean-branch.sh` +- **PR Template**: `./references/pr-template.md` +- **Commit Message Guide**: `./references/commit-message-guide.md` +- **Files to Exclude**: `./references/files-to-exclude.md` + +--- + +**Remember**: The goal is to make the maintainer's job as easy as possible. A well-prepared PR shows respect for their time and increases the likelihood of quick acceptance. diff --git a/references/pr-template.md b/references/pr-template.md new file mode 100644 index 0000000..f074f82 --- /dev/null +++ b/references/pr-template.md @@ -0,0 +1,266 @@ +# Pull Request Template + +Use this template when creating pull requests to open source projects. Adapt as needed based on the project's CONTRIBUTING.md guidelines. + +--- + +## Standard PR Template (What/Why/How) + +```markdown +## What? +[Brief, clear description of what this PR does - 1-2 sentences] + +## Why? +[Explain the reasoning, problem being solved, or business value - 2-3 sentences] + +## How? +[Describe the implementation approach and key decisions - bullet points work well] +- Key change 1 +- Key change 2 +- Key change 3 + +## Testing +[Step-by-step instructions for reviewers to test the changes] +1. Step 1 +2. Step 2 +3. Expected result + +## Checklist +- [ ] Tests added/updated +- [ ] Documentation updated +- [ ] CI passing +- [ ] Breaking changes documented (if any) +- [ ] Follows project code style + +## Related Issues +Closes #[issue number] +Relates to #[issue number] + +## Screenshots (if applicable) +[Add screenshots for visual changes, UI updates, or bug fixes] + +## Additional Notes +[Any other context, concerns, or questions for reviewers] +``` + +--- + +## Example: Feature Addition + +```markdown +## What? +Add OAuth2 authentication support for Google and GitHub providers + +## Why? +Users have requested social login to reduce friction during signup. This implements Key Result 2 of Q4 OKR1 and addresses multiple user requests from issue #234. + +## How? +- Implemented OAuth2 flow using passport.js strategy pattern +- Added provider configuration via environment variables +- Created callback routes for each provider (/auth/google/callback, /auth/github/callback) +- Updated user model to link social accounts with existing email-based accounts +- Added middleware to merge accounts if user already exists + +## Testing +1. Set up OAuth apps in Google and GitHub developer consoles +2. Add credentials to `.env` file (see `.env.example` for required variables) +3. Run `npm install` to ensure passport dependencies are installed +4. Start server: `npm start` +5. Navigate to `/login` +6. Click "Login with Google" button +7. Verify OAuth flow redirects correctly +8. Verify user profile data merges correctly with existing account (if applicable) +9. Test GitHub provider following same steps + +## Checklist +- [x] Tests added for OAuth flow (see tests/auth/oauth.test.js) +- [x] Documentation updated (see docs/authentication.md) +- [x] CI passing +- [x] No breaking changes +- [x] Follows existing code style +- [x] Environment variables documented in .env.example + +## Related Issues +Closes #234 +Relates to #156 (social login epic) + +## Screenshots +![OAuth Login Buttons](./screenshots/oauth-buttons.png) +*New social login buttons on login page* + +## Additional Notes +- Chose passport.js over custom implementation for security and maintenance +- Used strategy pattern to make adding new providers easier in future +- Account merging happens automatically based on email address +- Consider adding rate limiting for OAuth endpoints in future PR +``` + +--- + +## Example: Bug Fix + +```markdown +## What? +Fix memory leak in worker process shutdown + +## Why? +The worker pool wasn't properly cleaning up connections when shutting down gracefully, leading to memory leaks over time. This was causing production issues on long-running instances and reported in #456. + +## How? +- Added connection cleanup in worker shutdown handler +- Implemented timeout for graceful shutdown (30s default) +- Added mutex locks to prevent race conditions during cleanup +- Updated tests to verify proper cleanup + +## Testing +1. Run load test: `npm run test:load` +2. Monitor memory usage: `npm run monitor:memory` +3. Trigger graceful shutdown: `kill -SIGTERM ` +4. Verify memory is released (check monitor output) +5. Run for 24 hours to verify no gradual memory increase + +Manual testing: +- Tested on staging for 48 hours with production load +- Memory usage remained stable at ~120MB +- Previous behavior showed gradual increase to 2GB+ over 24h + +## Checklist +- [x] Tests added for shutdown cleanup +- [x] Documentation updated (shutdown behavior in README) +- [x] CI passing +- [x] No breaking changes +- [x] Tested on staging environment + +## Related Issues +Fixes #456 + +## Additional Notes +- Considered force-killing workers after timeout, but went with graceful degradation +- Mutex implementation follows existing pattern in connection-pool.js +- Backward compatible with existing shutdown handlers +``` + +--- + +## Example: Refactoring + +```markdown +## What? +Refactor authentication middleware to improve testability and reduce duplication + +## Why? +The authentication logic was duplicated across 5 different route handlers, making it difficult to test and maintain. This refactoring consolidates the logic into reusable middleware. + +## How? +- Extracted auth logic into `middleware/authenticate.js` +- Created composable middleware functions: + - `requireAuth()` - Basic authentication check + - `requireRole(role)` - Role-based access control + - `optionalAuth()` - Sets user if authenticated, continues if not +- Updated all routes to use new middleware +- Maintained backward compatibility with existing behavior + +## Testing +1. Run full test suite: `npm test` +2. Verify authentication still works: + - Try accessing protected route without token (should get 401) + - Access with valid token (should succeed) + - Access with invalid token (should get 401) +3. Verify role-based access: + - User role trying admin endpoint (should get 403) + - Admin role trying admin endpoint (should succeed) + +All existing tests pass without modification, demonstrating backward compatibility. + +## Checklist +- [x] All existing tests pass +- [x] Added tests for new middleware functions +- [x] Documentation updated +- [x] CI passing +- [x] No breaking changes +- [x] Code coverage maintained + +## Related Issues +Relates to #301 (technical debt epic) + +## Additional Notes +- No changes to API contracts - purely internal refactoring +- Reduces code duplication from ~200 lines to ~50 lines +- Future PRs can add new auth strategies more easily +- All 5 route handlers tested individually to verify behavior unchanged +``` + +--- + +## Example: Documentation Update + +```markdown +## What? +Update installation instructions for v2.0 and add troubleshooting section + +## Why? +Users have reported confusion about new installation requirements in v2.0. Multiple support requests (#567, #589, #601) asking about the same issues. + +## How? +- Updated README.md with new installation steps +- Added prerequisites section (Node.js 18+, npm 9+) +- Created troubleshooting guide for common issues +- Added examples for different deployment scenarios +- Fixed typos and outdated links + +## Testing +- Followed installation steps on fresh Ubuntu 22.04 VM +- Tested on macOS Ventura +- Verified all links work +- Asked non-technical user to follow guide (successfully completed) + +## Checklist +- [x] All links verified working +- [x] Code examples tested +- [x] Markdown formatting validated +- [x] Screenshots updated to v2.0 UI +- [x] Spelling and grammar checked + +## Related Issues +Closes #567, #589, #601 + +## Screenshots +N/A (documentation only) + +## Additional Notes +- Kept v1.0 migration guide in separate file (MIGRATION.md) +- Added FAQ section based on common support questions +- Consider adding video walkthrough in future +``` + +--- + +## Tips for Writing Good PR Descriptions + +### Do: +✅ Be specific and clear +✅ Explain WHY, not just WHAT +✅ Provide testing instructions +✅ Link related issues +✅ Include screenshots for visual changes +✅ Note breaking changes prominently +✅ Keep it concise but complete + +### Don't: +❌ Just repeat the commit message +❌ Leave sections empty ("TODO", "TBD") +❌ Assume reviewers know the context +❌ Include implementation details better suited for code comments +❌ Write a novel (keep it scannable) +❌ Skip testing instructions + +--- + +## Project-Specific Variations + +Some projects have specific PR template requirements. Check for: +- `.github/PULL_REQUEST_TEMPLATE.md` in the repository +- Instructions in CONTRIBUTING.md +- Examples in recently merged PRs + +**Always adapt this template to match project expectations.** diff --git a/scripts/clean-branch.sh b/scripts/clean-branch.sh new file mode 100755 index 0000000..fa9da0b --- /dev/null +++ b/scripts/clean-branch.sh @@ -0,0 +1,190 @@ +#!/bin/bash + +# Clean Branch Script +# Safely removes common personal development artifacts before PR submission +# Part of the open-source-contributions skill + +set -e + +# Colors for output +RED='\033[0;31m' +YELLOW='\033[1;33m' +GREEN='\033[0;32m' +BLUE='\033[0;34m' +NC='\033[0m' # No Color + +echo -e "${BLUE}====================================${NC}" +echo -e "${BLUE} Clean Branch for PR${NC}" +echo -e "${BLUE}====================================${NC}" +echo "" + +# Check if we're in a git repository +if ! git rev-parse --git-dir > /dev/null 2>&1; then + echo -e "${RED}✗ Not in a git repository${NC}" + exit 1 +fi + +# Interactive mode flag +INTERACTIVE=true +if [ "$1" = "--non-interactive" ] || [ "$1" = "-y" ]; then + INTERACTIVE=false +fi + +# Files to potentially remove +PERSONAL_FILES=( + "SESSION.md" + "NOTES.md" + "TODO.md" + "SCRATCH.md" + "DEBUGGING.md" + "TESTING.md" +) + +FOUND_FILES=() + +echo -e "${BLUE}Scanning for personal development artifacts...${NC}" +echo "" + +# Check for personal files +for file in "${PERSONAL_FILES[@]}"; do + if git ls-files --error-unmatch "$file" > /dev/null 2>&1; then + FOUND_FILES+=("$file") + echo -e "${YELLOW} Found: $file${NC}" + fi +done + +# Check for planning directory +if git ls-files | grep -q "^planning/"; then + FOUND_FILES+=("planning/") + echo -e "${YELLOW} Found: planning/* directory${NC}" + git ls-files | grep "^planning/" | head -5 | sed 's/^/ - /' + PLANNING_COUNT=$(git ls-files | grep "^planning/" | wc -l) + if [ "$PLANNING_COUNT" -gt 5 ]; then + echo -e "${YELLOW} ... and $((PLANNING_COUNT - 5)) more files${NC}" + fi +fi + +# Check for debug screenshots +DEBUG_SCREENSHOTS=$(git ls-files | grep -E "(debug|test|scratch).*\.(png|jpg|jpeg|gif)" || true) +if [ -n "$DEBUG_SCREENSHOTS" ]; then + echo -e "${YELLOW} Found: debug/test screenshots${NC}" + echo "$DEBUG_SCREENSHOTS" | sed 's/^/ - /' + FOUND_FILES+=("screenshots-debug") +fi + +# Check for temporary test files +TEMP_TESTS=$(git ls-files | grep -iE "(test-manual|test-debug|quick-test|scratch-test|example-local)" || true) +if [ -n "$TEMP_TESTS" ]; then + echo -e "${YELLOW} Found: temporary test files${NC}" + echo "$TEMP_TESTS" | sed 's/^/ - /' + FOUND_FILES+=("temp-tests") +fi + +echo "" + +# If nothing found, exit +if [ ${#FOUND_FILES[@]} -eq 0 ]; then + echo -e "${GREEN}✓ No personal artifacts found!${NC}" + echo -e "${GREEN}Your branch is clean.${NC}" + exit 0 +fi + +# Confirm removal +if [ "$INTERACTIVE" = true ]; then + echo -e "${YELLOW}These files should not be included in your PR.${NC}" + echo -e "${BLUE}Would you like to remove them? (y/n)${NC}" + read -r response + if [[ ! "$response" =~ ^[Yy]$ ]]; then + echo -e "${YELLOW}Cancelled. No files removed.${NC}" + exit 0 + fi +fi + +echo "" +echo -e "${BLUE}Removing files...${NC}" + +REMOVED_COUNT=0 + +# Remove personal files +for file in "${PERSONAL_FILES[@]}"; do + if git ls-files --error-unmatch "$file" > /dev/null 2>&1; then + git rm --cached "$file" 2>/dev/null || git rm "$file" 2>/dev/null || true + echo -e "${GREEN} ✓ Removed: $file${NC}" + ((REMOVED_COUNT++)) + fi +done + +# Remove planning directory +if git ls-files | grep -q "^planning/"; then + git rm --cached -r "planning/" 2>/dev/null || git rm -r "planning/" 2>/dev/null || true + echo -e "${GREEN} ✓ Removed: planning/* directory${NC}" + ((REMOVED_COUNT++)) +fi + +# Remove debug screenshots (interactive) +if [ -n "$DEBUG_SCREENSHOTS" ]; then + if [ "$INTERACTIVE" = true ]; then + echo "" + echo -e "${YELLOW}Found debug screenshots. Remove these too? (y/n)${NC}" + echo "$DEBUG_SCREENSHOTS" | sed 's/^/ - /' + read -r response + if [[ "$response" =~ ^[Yy]$ ]]; then + echo "$DEBUG_SCREENSHOTS" | while read -r file; do + git rm --cached "$file" 2>/dev/null || git rm "$file" 2>/dev/null || true + echo -e "${GREEN} ✓ Removed: $file${NC}" + done + ((REMOVED_COUNT++)) + fi + else + echo "$DEBUG_SCREENSHOTS" | while read -r file; do + git rm --cached "$file" 2>/dev/null || git rm "$file" 2>/dev/null || true + done + echo -e "${GREEN} ✓ Removed: debug screenshots${NC}" + ((REMOVED_COUNT++)) + fi +fi + +# Remove temporary test files (interactive) +if [ -n "$TEMP_TESTS" ]; then + if [ "$INTERACTIVE" = true ]; then + echo "" + echo -e "${YELLOW}Found temporary test files. Remove these too? (y/n)${NC}" + echo "$TEMP_TESTS" | sed 's/^/ - /' + read -r response + if [[ "$response" =~ ^[Yy]$ ]]; then + echo "$TEMP_TESTS" | while read -r file; do + git rm --cached "$file" 2>/dev/null || git rm "$file" 2>/dev/null || true + echo -e "${GREEN} ✓ Removed: $file${NC}" + done + ((REMOVED_COUNT++)) + fi + else + echo "$TEMP_TESTS" | while read -r file; do + git rm --cached "$file" 2>/dev/null || git rm "$file" 2>/dev/null || true + done + echo -e "${GREEN} ✓ Removed: temporary test files${NC}" + ((REMOVED_COUNT++)) + fi +fi + +echo "" +echo -e "${BLUE}====================================${NC}" + +if [ $REMOVED_COUNT -gt 0 ]; then + echo -e "${GREEN}✓ Cleaned $REMOVED_COUNT artifact(s)${NC}" + echo "" + echo -e "${BLUE}Next steps:${NC}" + echo -e " 1. Review changes: ${YELLOW}git status${NC}" + echo -e " 2. Commit removal: ${YELLOW}git commit -m 'chore: remove personal development artifacts'${NC}" + echo -e " 3. Re-run check: ${YELLOW}./scripts/pre-pr-check.sh${NC}" + echo -e " 4. Push changes: ${YELLOW}git push${NC}" +else + echo -e "${GREEN}✓ No files removed${NC}" +fi + +echo "" +echo -e "${YELLOW}Note:${NC} Files are removed from git tracking but still exist locally." +echo -e "Add them to .git/info/exclude to prevent re-adding:" +echo "" +echo -e "${BLUE}echo 'SESSION.md' >> .git/info/exclude${NC}" +echo -e "${BLUE}echo 'planning/' >> .git/info/exclude${NC}" diff --git a/scripts/pre-pr-check.sh b/scripts/pre-pr-check.sh new file mode 100755 index 0000000..4229118 --- /dev/null +++ b/scripts/pre-pr-check.sh @@ -0,0 +1,249 @@ +#!/bin/bash + +# Pre-PR Check Script +# Scans for common personal development artifacts before PR submission +# Part of the open-source-contributions skill + +set -e + +# Colors for output +RED='\033[0;31m' +YELLOW='\033[1;33m' +GREEN='\033[0;32m' +BLUE='\033[0;34m' +NC='\033[0m' # No Color + +echo -e "${BLUE}====================================${NC}" +echo -e "${BLUE} Pre-PR Validation Check${NC}" +echo -e "${BLUE}====================================${NC}" +echo "" + +ERRORS=0 +WARNINGS=0 + +# Check if we're in a git repository +if ! git rev-parse --git-dir > /dev/null 2>&1; then + echo -e "${RED}✗ Not in a git repository${NC}" + exit 1 +fi + +# Get current branch +CURRENT_BRANCH=$(git rev-parse --abbrev-ref HEAD) +echo -e "${BLUE}Current branch:${NC} $CURRENT_BRANCH" +echo "" + +# Check 1: Personal Development Artifacts +echo -e "${BLUE}[1/8] Checking for personal development artifacts...${NC}" + +PERSONAL_FILES=( + "SESSION.md" + "NOTES.md" + "TODO.md" + "SCRATCH.md" + "DEBUGGING.md" + "TESTING.md" +) + +for file in "${PERSONAL_FILES[@]}"; do + if git ls-files --error-unmatch "$file" > /dev/null 2>&1; then + echo -e "${RED} ✗ Found: $file (should not be in PR)${NC}" + ((ERRORS++)) + fi +done + +# Check planning directory +if git ls-files | grep -q "^planning/"; then + echo -e "${RED} ✗ Found: planning/* directory (should not be in PR)${NC}" + git ls-files | grep "^planning/" | head -5 | sed 's/^/ - /' + ((ERRORS++)) +fi + +if [ $ERRORS -eq 0 ]; then + echo -e "${GREEN} ✓ No personal development artifacts found${NC}" +fi +echo "" + +# Check 2: Screenshots and Visual Assets +echo -e "${BLUE}[2/8] Checking for debug screenshots...${NC}" + +DEBUG_SCREENSHOTS=$(git ls-files | grep -E "screenshot.*\.(png|jpg|jpeg|gif|mp4|mov)" || true) +if [ -n "$DEBUG_SCREENSHOTS" ]; then + echo -e "${YELLOW} ⚠ Found screenshots:${NC}" + echo "$DEBUG_SCREENSHOTS" | sed 's/^/ - /' + echo -e "${YELLOW} → Ensure these are needed for PR description (demonstrating feature)${NC}" + echo -e "${YELLOW} → Remove if they're just debugging artifacts${NC}" + ((WARNINGS++)) +else + echo -e "${GREEN} ✓ No screenshot files found${NC}" +fi +echo "" + +# Check 3: Temporary Test Files +echo -e "${BLUE}[3/8] Checking for temporary test files...${NC}" + +TEMP_TEST_PATTERNS=( + "test-manual" + "test-debug" + "quick-test" + "scratch-test" + "example-local" + "debug-" + "temp-" + "tmp-" +) + +TEMP_TESTS_FOUND=0 +for pattern in "${TEMP_TEST_PATTERNS[@]}"; do + MATCHES=$(git ls-files | grep -i "$pattern" || true) + if [ -n "$MATCHES" ]; then + echo -e "${RED} ✗ Found temporary test files matching '$pattern':${NC}" + echo "$MATCHES" | sed 's/^/ - /' + ((ERRORS++)) + TEMP_TESTS_FOUND=1 + fi +done + +if [ $TEMP_TESTS_FOUND -eq 0 ]; then + echo -e "${GREEN} ✓ No temporary test files found${NC}" +fi +echo "" + +# Check 4: Large Files +echo -e "${BLUE}[4/8] Checking for large files (>1MB)...${NC}" + +LARGE_FILES=$(git ls-files | while read file; do + if [ -f "$file" ]; then + size=$(stat -c%s "$file" 2>/dev/null || stat -f%z "$file" 2>/dev/null) + if [ "$size" -gt 1048576 ]; then + echo "$file ($(numfmt --to=iec-i --suffix=B $size 2>/dev/null || echo "$((size / 1024))KB"))" + fi + fi +done) + +if [ -n "$LARGE_FILES" ]; then + echo -e "${YELLOW} ⚠ Found large files:${NC}" + echo "$LARGE_FILES" | sed 's/^/ - /' + echo -e "${YELLOW} → Ensure these are necessary for the PR${NC}" + ((WARNINGS++)) +else + echo -e "${GREEN} ✓ No large files found${NC}" +fi +echo "" + +# Check 5: Potential Secrets +echo -e "${BLUE}[5/8] Checking for potential secrets...${NC}" + +SECRET_PATTERNS=( + "\.env$" + "\.env\.local$" + "credentials\.json$" + "\.key$" + "\.pem$" + "secret" + "password" + "api[_-]?key" + "access[_-]?token" +) + +SECRETS_FOUND=0 +for pattern in "${SECRET_PATTERNS[@]}"; do + MATCHES=$(git ls-files | grep -iE "$pattern" || true) + if [ -n "$MATCHES" ]; then + echo -e "${RED} ✗ Found potential secrets matching '$pattern':${NC}" + echo "$MATCHES" | sed 's/^/ - /' + ((ERRORS++)) + SECRETS_FOUND=1 + fi +done + +if [ $SECRETS_FOUND -eq 0 ]; then + echo -e "${GREEN} ✓ No potential secrets found in file names${NC}" +fi +echo "" + +# Check 6: PR Size +echo -e "${BLUE}[6/8] Checking PR size...${NC}" + +# Get default branch (usually main or master) +DEFAULT_BRANCH=$(git remote show origin | grep 'HEAD branch' | cut -d' ' -f5) +if [ -z "$DEFAULT_BRANCH" ]; then + DEFAULT_BRANCH="main" +fi + +# Count lines changed +if git rev-parse --verify "origin/$DEFAULT_BRANCH" > /dev/null 2>&1; then + LINES_CHANGED=$(git diff --shortstat "origin/$DEFAULT_BRANCH" | grep -oE '[0-9]+ insertion|[0-9]+ deletion' | grep -oE '[0-9]+' | paste -sd+ | bc || echo "0") + FILES_CHANGED=$(git diff --name-only "origin/$DEFAULT_BRANCH" | wc -l) + + echo -e "${BLUE} Lines changed:${NC} $LINES_CHANGED" + echo -e "${BLUE} Files changed:${NC} $FILES_CHANGED" + + if [ "$LINES_CHANGED" -lt 50 ]; then + echo -e "${GREEN} ✓ Small PR (< 50 lines) - Ideal!${NC}" + elif [ "$LINES_CHANGED" -lt 200 ]; then + echo -e "${GREEN} ✓ Good PR size (< 200 lines)${NC}" + elif [ "$LINES_CHANGED" -lt 400 ]; then + echo -e "${YELLOW} ⚠ Large PR (< 400 lines) - Consider splitting if possible${NC}" + ((WARNINGS++)) + else + echo -e "${RED} ✗ Very large PR (>= 400 lines) - Strongly consider splitting${NC}" + ((ERRORS++)) + fi +else + echo -e "${YELLOW} ⚠ Cannot compare with origin/$DEFAULT_BRANCH${NC}" + ((WARNINGS++)) +fi +echo "" + +# Check 7: Uncommitted Changes +echo -e "${BLUE}[7/8] Checking for uncommitted changes...${NC}" + +if ! git diff-index --quiet HEAD --; then + echo -e "${YELLOW} ⚠ You have uncommitted changes${NC}" + git status --short | sed 's/^/ /' + echo -e "${YELLOW} → Commit or stash these before creating PR${NC}" + ((WARNINGS++)) +else + echo -e "${GREEN} ✓ No uncommitted changes${NC}" +fi +echo "" + +# Check 8: Branch Name +echo -e "${BLUE}[8/8] Checking branch name...${NC}" + +if [ "$CURRENT_BRANCH" = "main" ] || [ "$CURRENT_BRANCH" = "master" ]; then + echo -e "${RED} ✗ You're on $CURRENT_BRANCH branch!${NC}" + echo -e "${RED} → Never create PRs from main/master${NC}" + echo -e "${RED} → Create a feature branch instead${NC}" + ((ERRORS++)) +else + echo -e "${GREEN} ✓ On feature branch: $CURRENT_BRANCH${NC}" +fi +echo "" + +# Summary +echo -e "${BLUE}====================================${NC}" +echo -e "${BLUE} Summary${NC}" +echo -e "${BLUE}====================================${NC}" + +if [ $ERRORS -eq 0 ] && [ $WARNINGS -eq 0 ]; then + echo -e "${GREEN}✓ All checks passed!${NC}" + echo -e "${GREEN}Your branch is ready for PR submission.${NC}" + exit 0 +elif [ $ERRORS -eq 0 ]; then + echo -e "${YELLOW}⚠ $WARNINGS warning(s) found${NC}" + echo -e "${YELLOW}Review warnings above and address if needed.${NC}" + exit 0 +else + echo -e "${RED}✗ $ERRORS error(s) found${NC}" + if [ $WARNINGS -gt 0 ]; then + echo -e "${YELLOW}⚠ $WARNINGS warning(s) found${NC}" + fi + echo -e "${RED}Fix errors above before submitting PR.${NC}" + echo "" + echo -e "${BLUE}Next steps:${NC}" + echo -e " 1. Remove personal artifacts: ${YELLOW}git rm --cached ${NC}" + echo -e " 2. Clean branch: ${YELLOW}./scripts/clean-branch.sh${NC}" + echo -e " 3. Re-run this check: ${YELLOW}./scripts/pre-pr-check.sh${NC}" + exit 1 +fi