Git Workflow Best Practices | Branching, PR Reviews, Conventional Commits & CI

Branching Strategy

GitHub Flow (Recommended for Most Teams)

main (always deployable)
  ├── feature/user-authentication
  ├── fix/login-redirect-bug
  └── chore/update-dependencies

# 1. Create branch from main
git checkout main && git pull
git checkout -b feature/user-authentication

# 2. Make commits
git add .
git commit -m "feat(auth): add JWT token validation"

# 3. Push and open PR
git push -u origin feature/user-authentication
gh pr create --title "Add user authentication" --body "..."

# 4. Review, fix, merge to main
# (merge/squash/rebase in GitHub UI)

# 5. Delete branch after merge
git branch -d feature/user-authentication

Trunk-Based Development (High-frequency Delivery)

main (commit directly or very short-lived branches)
  ← feature flags hide incomplete features
  ← CI must pass on every commit
  ← deploy from every green main commit

Good when: strong CI/CD culture, feature flags, frequent deployments.

---

Branch Naming

# Format: type/short-description
feature/user-authentication
feature/payment-stripe-integration
fix/login-redirect-loop
fix/api-rate-limit-headers
hotfix/critical-sql-injection
chore/upgrade-react-18
docs/api-authentication-guide
refactor/extract-payment-service
test/add-checkout-integration-tests

# With ticket reference
feature/JIRA-123-user-authentication
fix/GH-456-login-redirect

# Rules:
# - lowercase, hyphens not underscores
# - short but descriptive (3-5 words)
# - prefix with type

---

Conventional Commits

Format: type(scope): description

          feat: new feature (triggers minor version bump)
          fix:  bug fix (triggers patch version bump)
          docs: documentation changes only
      refactor: code change that neither fixes bug nor adds feature
          test: adding or updating tests
         chore: build, CI, dependency updates
         style: formatting, no logic change
          perf: performance improvement
          revert: revert a previous commit

         (scope): optional, names the module affected
         BREAKING CHANGE: footer triggers major version bump

Examples:

git commit -m "feat(auth): add Google OAuth login"
git commit -m "fix(api): handle null response from payment provider"
git commit -m "docs(readme): add Docker setup instructions"
git commit -m "refactor(database): extract connection pooling to module"
git commit -m "test(auth): add JWT expiry edge case tests"
git commit -m "chore: upgrade TypeScript to 5.4"
git commit -m "perf(search): add index on posts.created_at"

# Breaking change (triggers major version bump)
git commit -m "feat(api)!: change user ID from integer to UUID

BREAKING CHANGE: User IDs are now UUIDs (string) instead of integers.
All API consumers must update their client code."

---

Commit Quality

# ✅ Good commits:
feat(auth): add JWT refresh token rotation
fix(cart): prevent duplicate items when clicking Add to Cart twice
refactor(user): extract address validation to AddressValidator class
test(payment): add integration tests for Stripe webhook handling

# ❌ Bad commits:
WIP
fix stuff
update
asdf
changes
more work on the thing
.

Good commit principles:

• Completes one logical change (atomic)
• First line ≤ 72 characters
• Describes what and why, not how
• Tests pass (don’t commit broken code)
• If you need “and” in the message — consider two commits

---

PR Best Practices

Writing a Good PR Description

## What

Add JWT refresh token rotation to the auth system.

## Why

Currently, access tokens don't expire — a stolen token grants permanent access.
This PR adds 15-minute access tokens with refresh token rotation.

## Changes

- `AuthService.login()` now returns both access and refresh tokens
- `AuthService.refresh()` validates refresh token and issues new pair
- Refresh tokens are stored in HttpOnly cookies (not localStorage)
- Added `POST /auth/refresh` endpoint

## Testing

- Run `npm test` — all 47 auth tests pass
- Manual: login → wait 15min → verify auto-refresh works
- Edge case: revoked refresh token → verify 401 returned

## Notes

The refresh token table migration runs automatically in deployment.
No manual steps needed.

PR Size

Ideal PR size: 200-400 lines changed
Maximum for reasonable review: ~800 lines

Too large? Split by:
  - Component/layer: API changes in one PR, UI in another
  - Feature flag: merge implementation behind flag, enable in follow-up
  - Stacked PRs: PR2 targets PR1's branch (merged together)

Large PRs get rubber-stamp reviews — the cognitive load is too high
for thorough review past ~500 lines.

Review Checklist

Before requesting review:
  [ ] Self-review your own diff (you'll catch obvious issues)
  [ ] Tests pass locally
  [ ] No debug console.log left
  [ ] No TODO comments left (or they're tracked in issues)
  [ ] No sensitive data (passwords, tokens, keys)
  [ ] PR description explains the why

When reviewing:
  [ ] Does it solve the stated problem?
  [ ] Are there edge cases not handled?
  [ ] Is the code readable? Would I understand it in 6 months?
  [ ] Are there security implications?
  [ ] Do the tests actually test the right things?
  [ ] Does it follow team conventions?

---

Merge Strategies

# Strategy 1: Squash merge (recommended for most teams)
# All PR commits → 1 commit on main
git merge --squash feature/user-auth
git commit -m "feat(auth): add user authentication (#123)"
# Main history: one commit per feature, clean and readable

# Strategy 2: Rebase merge (linear history, all commits)
git rebase main feature/user-auth
git checkout main && git merge feature/user-auth  # Fast-forward
# Main history: all commits, but no merge commits

# Strategy 3: Regular merge (merge commit)
git merge feature/user-auth
# Creates: "Merge pull request #123 from feature/user-auth"
# History shows when branches were integrated

---

Protected Branches & Branch Rules

# GitHub: Settings → Branches → Branch protection rules

Branch name pattern: main

Rules:
  ✅ Require pull request reviews before merging (1 reviewer min)
  ✅ Dismiss stale pull request approvals when new commits are pushed
  ✅ Require status checks to pass before merging (CI)
  ✅ Require branches to be up to date before merging
  ✅ Restrict who can push to matching branches
  ✅ Require linear history (prevents merge commits)
  ✅ Require signed commits (GPG)

---

Commit Signing (GPG)

# Generate GPG key
gpg --full-generate-key    # RSA, 4096 bits, no expiry

# Get key ID
gpg --list-secret-keys --keyid-format=long
# sec   rsa4096/3AA5C34371567BD2 2024-01-01

# Configure Git to sign commits
git config --global user.signingkey 3AA5C34371567BD2
git config --global commit.gpgsign true  # Sign all commits

# Export public key → add to GitHub account
gpg --armor --export 3AA5C34371567BD2

# Verify: signed commits show "Verified" badge on GitHub

---

.gitignore Essentials

# .gitignore

# Secrets — NEVER commit
.env
.env.local
.env.*.local
*.pem
*.key
credentials.json

# Dependencies
node_modules/
venv/
.venv/
__pycache__/

# Build output
dist/
build/
.next/
out/

# IDE
.vscode/settings.json    # Project settings OK, personal settings no
.idea/
*.swp

# OS
.DS_Store
Thumbs.db

# Logs
*.log
npm-debug.log*

---

Useful Git Aliases

# Add to ~/.gitconfig
[alias]
  lg = log --oneline --graph --decorate --all
  st = status -sb
  co = checkout
  br = branch
  cp = cherry-pick
  undo = reset HEAD~1 --mixed   # Undo last commit, keep changes staged
  wip = !git add -A && git commit -m "WIP"
  unwip = !git log -n 1 | grep -q WIP && git reset HEAD~1

# Usage
git lg                    # Visual branch graph
git undo                  # Undo last commit (keep changes)
git wip                   # Quick WIP commit

---

CI/CD Integration

# .github/workflows/ci.yml
on:
  pull_request:
    branches: [main]
  push:
    branches: [main]

jobs:
  ci:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
      - uses: actions/setup-node@v4
        with:
          node-version: 20
          cache: npm
      - run: npm ci
      - run: npm run lint         # Code style
      - run: npm run type-check   # TypeScript
      - run: npm run test:run     # Unit tests
      - run: npm run build        # Production build

# ✅ PR can only merge when all checks pass
# ✅ Catches issues before they reach main
# ✅ Reviewers focus on logic, not style (linting handles that)

Related posts:

• Git Interactive Rebase Guide
• GitHub Actions CI/CD Guide