DEV Community

Cover image for How We Built a Code Quality Pipeline That Roasts Bad PRs Before Humans Have To
Keshav Agarwal
Keshav Agarwal

Posted on

How We Built a Code Quality Pipeline That Roasts Bad PRs Before Humans Have To

#github #devops #eslint #react

The Slow Descent Into "It's Fine, Ship It"

Nobody wakes up one day and decides to ruin a codebase. It's more of a group activity that happens gradually, like a potluck where everyone brings potato salad.

When our team was small — two or three devs who reviewed every line — code quality maintained itself through osmosis. We all knew the patterns. We all agreed on formatting. Nobody wrote an 800-line component because someone would notice and gently question their life choices.

Then the team grew. More features, more devs, more PRs flying in simultaneously. And the cracks started showing:

  • Components quietly ballooning past 700 lines — because "I'll refactor it later" (narrator: they did not refactor it later)
  • Formatting roulette: tabs here, spaces there, trailing commas in this file but not that one
  • Duplicate i18n messages across different feature modules — the same English string defined in three different messages.js files with three different IDs, leading to translation mismatches and wasted localization budget (more on this — it's a bigger deal than it sounds)
  • console.log('HERE') statements left behind like breadcrumbs from a debugging session that ended three sprints ago
  • var declarations showing up in a modern codebase, haunting us like a ghost from JavaScript past

No single dramatic incident. No production outage. Just a slow, steady erosion of the standards we thought were obvious. Death by a thousand "LGTM, merge it" reviews.

Code review alone couldn't save us — reviewers are human, PRs are long, and "does this file exceed 500 lines?" isn't the kind of thing you catch at 11 PM after your third coffee. We needed robots. Specifically, mean robots that block your PR and make you feel bad about your formatting choices.

"But What About CodeRabbit / AI Review Tools?"

Fair question. We use CodeRabbit for PR reviews and it's great at catching logic issues and suggesting improvements. But on the free tiers, it suggests fixes — it doesn't enforce them. A developer can read CodeRabbit’s “you should fix this formatting” comment, nod thoughtfully… and merge anyway.

We needed something with teeth. Something that physically prevents a merge until the code meets the bar. So we built our own enforcement layer with GitHub Actions.

"Why Not Biome / oxlint / Modern Linters?"

Also a fair question. New-gen linters like Biome are blazing fast and genuinely exciting. But our build pipelines are still on older Node versions, and our entire toolchain — Babel plugins, Webpack config, ESLint plugins for Redux-Saga, styled-components accessibility — is deeply integrated with the ESLint ecosystem. Migrating to Biome would mean rebuilding a lot of that plugin infrastructure.

ESLint isn't the shiny new thing, but it's battle-tested, extensible, and — critically — it works with our current stack without a rewrite. Sometimes the boring tool that ships today beats the exciting tool that requires a migration quarter.

The Architecture: The Quality Gauntlet

We ended up with a layered quality pipeline. Each layer catches different things at different stages, so by the time code reaches master, it's been interrogated more thoroughly than a suspect in a crime drama.

┌──────────────────────────────────────────────────────────────┐
│                    QUALITY PIPELINE                          │
│                 (a.k.a. "The Gauntlet")                      │
├──────────────────────────────────────────────────────────────┤
│                                                              │
│  The Foundation: THE RULEBOOK                                │
│  ┌────────────────────────────────────────────────┐          │
│  │  .eslintrc.yml — the opinionated ruleset       │          │
│  │  max-lines, complexity, prop-types, camelcase  │          │
│  │  + smart overrides for reducers/sagas/tests    │          │
│  └────────────────────────────────────────────────┘          │
│         │ powers both layers below                           │
│         v                                                    │
│  Layer 1: COMMIT TIME (local)                                │
│  ┌────────────────────────────────────────────────┐          │
│  │  lint-staged                                   │          │
│  │  • ESLint --fix on staged *.js files           │          │
│  │  • Prettier on staged *.json files             │          │
│  │  Catches: formatting, auto-fixable errors      │          │
│  └────────────────────────────────────────────────┘          │
│         │                                                    │
│         v                                                    │
│  Layer 2: PR TIME (GitHub Actions — run in parallel)         │
│  ┌──────────────┐ ┌─────────────────┐ ┌───────────┐          │
│  │ JS Linter    │ │ Duplicate i18n  │ │ Gitleaks  │          │
│  │ (full ESLint)│ │ (cross-file)    │ │ (secrets) │          │
│  └──────┬───────┘ └───────┬─────────┘ └─────┬─────┘          │
│         └────────────┬────┘                  │               │
│                      v                       │               │
│              Merge blocked until ALL pass ◄───┘              │
│                                                              │
└──────────────────────────────────────────────────────────────┘
Enter fullscreen mode Exit fullscreen mode

Each layer exists because the previous one isn't enough. The ESLint rulebook defines what matters — it's the foundation that powers everything else. lint-staged catches formatting but only runs on staged files. GitHub Actions catches everything but only on PRs. All three together form the net. One layer alone is a suggestion. Three layers together are the law.

The PR checks run in parallel — the linter, duplicate i18n check, and Gitleaks scan are independent workflows that kick off simultaneously. Since they don't share state, GitHub runs them on separate runners at the same time. For most repos, the entire suite finishes in under a minute. You push, you wait a few seconds, and you know exactly where you stand.

Layer 1: Pre-Commit Hooks (lint-staged)

The first line of defense runs before code ever leaves your machine:

{
  "lint-staged": {
    "*.js": "npm run lint:eslint:fix",
    "*.json": "prettier --write"
  },
  "pre-commit": "lint:staged"
}
Enter fullscreen mode Exit fullscreen mode

Every git commit triggers this. JavaScript files get ESLint with --fix — formatting issues, missing semicolons, trailing spaces are auto-corrected before you even see them. JSON files get Prettier. The developer doesn't have to remember anything. The formatting wars are over before they start.

Note: If you're on an older version of lint-staged (< v10), you'll need to add "git add --force" after each command. v10+ handles re-staging automatically.

But lint-staged only touches staged files. If ComponentA.js is clean but ComponentB.js already has a linting error, lint-staged won't catch it. Also, a developer can bypass this entirely with git commit --no-verify. (We trust our team, but we also trust automation more.)

That's where the real enforcers come in.

Layer 2: GitHub Actions — The PR Gatekeepers

The Linter Workflow

Every PR triggers a full ESLint pass across the entire codebase. Here's the workflow (condensed — the real one installs ~30 packages):

name: JavaScript Linting Validator

on:
  pull_request:
    types: [opened, reopened, synchronize]
    branches:
      - '*'

jobs:
  js-lint:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
      - uses: actions/setup-node@v4
        with:
          node-version: 'lts/*'

      # Nuclear option: clean slate every time
      - run: rm -f .npmrc
      - run: rm -rf node_modules package-lock.json
      - run: npm cache clean --force

      # Install only linting deps (not the whole project)
      - name: Install ESLint + Prettier + Babel deps
        run: |
          npm install "@babel/core@^7.26.7" \
            "@babel/eslint-parser@^7.26.5" \
            "eslint@^8.57.0" \
            "eslint-config-prettier@^10.0.1" \
            "eslint-plugin-react@^7.37.4" \
            "eslint-plugin-prettier@^5.2.3" \
            "prettier@^3.4.2" \
            # ... ~20 more packages
            --no-save --silent

      - run: npx eslint .
Enter fullscreen mode Exit fullscreen mode

A few things worth calling out:

The clean-slate install. We nuke node_modules, package-lock.json, .npmrc, and the npm cache before installing. Aggressive? Yes. It ensures we're never tripped up by stale local artifacts. One tradeoff worth noting: deleting the lockfile means CI resolves versions independently from your local setup, so there's a slim chance of version drift between what devs run locally and what CI installs. For linting-only deps this is a low risk we accept — but for production installs, you'd want to keep the lockfile.

Every PR, every branch. Not just master — every branch. If your feature branch has a lint error, you know immediately.

It blocks the merge. This is a required check. No "I'll fix it in the next PR" escape hatch. The robot says no, the robot means no.

The Duplicate i18n Check (Our Favorite)

This one solves a problem that quietly costs real money at scale, and we haven't seen many teams talk about it.

The problem: In a large internationalized codebase, many features define their own messages.js file for react-intl. Over time, different developers writing similar features independently define the same English string:

// In CreateEntity/messages.js
defineMessages({
  saveButton: { id: 'app.createEntity.save', defaultMessage: 'Save' },
});

// In EditEntity/messages.js (different dev, different sprint)
defineMessages({
  saveAction: { id: 'app.editEntity.save', defaultMessage: 'Save' },
});
Enter fullscreen mode Exit fullscreen mode

Two different IDs, same defaultMessage. In English, no one notices. But when you send these to a translation vendor, you're paying to translate the same string multiple times. Worse — different translators might translate "Save" differently, so users see inconsistent text across the product. One screen says the equivalent of "Store" and another says "Keep." Subtle, but it erodes trust.

At scale — dozens of languages, hundreds of message strings — these duplicates add up fast in translation costs and QA time.

The workflow is dead simple:

name: Check Duplicate i18n Messages

on:
  pull_request:
    types: [opened, reopened, synchronize]
    branches: ['*']

jobs:
  check-duplicates:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
      - uses: actions/setup-node@v4
        with:
          node-version: 'lts/*'
      - run: node scripts/check-duplicate-messages.js
Enter fullscreen mode Exit fullscreen mode

The custom script behind it:

  1. Recursively finds every messages.js and message.js file
  2. Regex-extracts all { id: '...', defaultMessage: '...' } definitions — handling single-line, multi-line, and template literal formats
  3. Builds a map of message values to their file locations
  4. Flags any defaultMessage that appears in two or more different files

When it fails, the output is clear:

Found 47 message files to check

CROSS-FILE DUPLICATE MESSAGE VALUE FOUND
Value: "Save"
  In app/components/pages/CreateEntity/messages.js:
    - ID: app.createEntity.save
  In app/components/pages/EditEntity/messages.js:
    - ID: app.editEntity.save

Cross-file duplicate message values found.
Enter fullscreen mode Exit fullscreen mode

PR blocked. Fix is always the same: extract the shared string to a common messages file or reuse the existing definition. This single check has saved us from dozens of translation inconsistencies — and a non-trivial amount of translation spend.

Gitleaks Secret Scan

We also run Gitleaks on every PR to master. It scans for accidentally committed secrets — API keys, tokens, credentials:

name: Gitleaks Secret Scan

on:
  pull_request:
    types: [opened, reopened, synchronize]
    branches: [master]

jobs:
  gitleaks:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
        with:
          fetch-depth: 0
      - uses: gitleaks/gitleaks-action@v2
        env:
          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
Enter fullscreen mode Exit fullscreen mode

Standard security hygiene. If you're not doing this, start today. It catches the kind of mistake that turns a normal Tuesday into an incident response.

The Rulebook: ESLint Rules That Actually Matter

The rules are the backbone. Anyone can set up a linter — the interesting part is what you choose to enforce. Here's our opinionated ruleset with the "why" behind each choice.

max-lines: 500 — The God Component Killer 

max-lines:
  - error
  - max: 500
Enter fullscreen mode Exit fullscreen mode

500 lines is generous enough for complex forms but strict enough to prevent the 800-line monsters that nobody wants to review and everybody pretends they'll "refactor someday." If your component hits 500 lines, it's doing too much. Split it. Your future self (and your reviewers) will thank you.

complexity: 10 — Keep Functions Readable 

complexity:
  - error
  - max: 10
Enter fullscreen mode Exit fullscreen mode

Cyclomatic complexity of 10 means a function can have at most 10 independent paths. This catches those deeply nested if/else/switch pyramids that look like they were written during a fever dream. If your function has complexity 15, it needs decomposing.

react/prop-types: error — Component Contracts 

react/prop-types: error
react/require-default-props: error
Enter fullscreen mode Exit fullscreen mode

Yes, we know TypeScript exists. We're not using it (yet). PropTypes are the closest thing to a component contract in JS-land. When a new dev opens a component, propTypes tells them exactly what it expects — no reading the entire render method to figure out what props exist.

camelcase: error — One Convention to Rule Them All 

camelcase:
  - error
  - properties: always
Enter fullscreen mode Exit fullscreen mode

Try reading a codebase where half the variables are user_name and the other half are userName. It's like reading a book where the author randomly switches between British and American spelling. Technically functional, spiritually painful.

prettier/prettier: error — Formatting Is Not a Discussion 

prettier/prettier:
  - error
  - { singleQuote: true, printWidth: 100, endOfLine: 'lf' }
Enter fullscreen mode Exit fullscreen mode

Prettier as an ESLint error means formatting violations block the merge. Not warnings. Not suggestions. The specific settings (single quotes, 100-char lines) matter less than the fact that they're enforced uniformly. Tabs vs spaces debates belong in 2015 where we left them.

The Smart Part: Overrides

Not every file should follow the same rules. This is where the config stops being dogmatic and starts being practical:

overrides:
  - files: '**/reducer.js'
    rules:
      complexity: off

  - files: '**/saga.js'
    rules:
      complexity: off
      camelcase: off

  - files: '**/*.test.js'
    rules:
      react/prop-types: off

  - files: '**/tests/**/*.test.js'
    rules:
      complexity: off
      camelcase: off
      max-lines: off
Enter fullscreen mode Exit fullscreen mode

Reducers get unlimited complexity because Redux reducers are fundamentally big switch statements. A reducer handling 15 actions has complexity 15 — that's not bad code, that's just how reducers work. Forcing artificial splits would make the code worse.

Sagas get complexity + camelcase exemptions because sagas are the boundary layer where external API naming (user_name, created_at) meets internal conventions. Forcing camelcase there means either renaming every API field or suppressing the rule on every line. Both are worse than the override.

Tests get relaxed rules because we want developers to write more tests, not fewer. If prop-types or max-lines are blocking test coverage, the rules are hurting more than helping.

What We'd Do Differently

Honesty hour.

Outdated Node in CI. Our linter workflow was pinned to an older Node version for a while — the kind of tech debt that works fine until it doesn't, like driving with an expired inspection sticker. Upgrading means verifying 30+ Babel and ESLint packages resolve correctly on the newer runtime. We got there eventually, but it sat on the "we'll get to it" list longer than we'd like to admit.

The clean-slate install is slow. Nuking node_modules and reinstalling from scratch on every PR isn't fast. Dependency caching (actions/cache) would fix this. We traded speed for reproducibility, but you can have both.

The Takeaway

If you're setting up quality gates for a multi-developer codebase, here's the framework:

  1. Start with the rules. Define what "good code" means in your linter config. Be opinionated — it's easier to relax rules than to introduce them into a codebase that's already feral.

  2. Enforce locally. lint-staged + pre-commit hooks fix the easy stuff automatically. Zero friction.

  3. Enforce in CI. GitHub Actions catch what local hooks miss — and unlike pre-commit hooks, they can't be skipped with --no-verify.

  4. Write overrides, not exceptions. Instead of // eslint-disable-next-line scattered across hundreds of files, configure overrides by file pattern. Encode your team's decisions in the config, not in comments.

  5. Build custom checks for your pain. The duplicate i18n check isn't in any plugin — we wrote it because it was our problem. Every codebase has unique failure modes. Write scripts to catch yours.

The goal isn't zero lint warnings. The goal is that a new developer can join the team, open a PR, and the pipeline tells them exactly how the team writes code — without anyone writing a Confluence doc that nobody reads.

Running custom quality checks in your CI that go beyond standard linting? I'd love to hear what problems they solve. Drop them in the comments.

Top comments (0)