Atlas

Prácticas de ingeniería: code review, CI/CD, release, estándares

Las cuatro prácticas que separan a un equipo que “escribe código” de uno que entrega software de forma predecible y segura: cómo se revisa, cómo se integra, cómo se libera y cómo se estandariza. Nivel senior: no solo usarlas, sino diseñarlas, establecerlas en un contexto dado y detectar sus cuellos de botella.

Estas prácticas forman un lazo cerrado. El code review captura defectos y difunde conocimiento antes de mergear; el CI convierte cada merge en una verificación automática; la release strategy decide cómo ese código verificado llega a producción con riesgo acotado; y los code standards son el contrato objetivo que hace que review y CI puedan automatizarse en vez de discutirse. Un senior no las trata como cajas separadas: diseña un pipeline donde los estándares se vuelven quality gates, el review se apoya en esos gates para enfocarse en lo que las máquinas no ven, y la estrategia de release consume el artifact que salió del pipeline.


Teoría

Code Review

El code review es una actividad de aseguramiento de calidad donde una o más personas leen el código de un cambio antes (o poco después) de integrarlo. Sus objetivos combinados: mejor calidad de código, detección de defectos, transferencia de conocimiento y sentido de responsabilidad compartida. A nivel senior lo relevante no es “saber revisar”, sino establecer la práctica: definir aims, feedback, reporting, periodicity y jerarquía de reviewers, y crear checklists específicos del proyecto.

Pre-commit vs post-commit review. El modelo dominante hoy es pre-merge (pull/merge request): el cambio no entra a la rama principal hasta que un reviewer aprueba. Es la forma más estricta y la que habilita quality gates bloqueantes. El post-commit (revisar después de integrar, estilo commit-then-review) reduce fricción y acelera trunk-based development, pero exige mucha disciplina y buena cobertura de CI porque el código defectuoso ya está en main. Un senior elige según la madurez del equipo: post-commit + CI fuerte para equipos maduros que hacen deploy continuo; pre-merge estricto para equipos nuevos o dominios de alto riesgo.

Qué debe buscar un reviewer (jerarquía de importancia). El error clásico del review junior es gastar toda la atención en estilo. Lo que las máquinas ya cubren (formato, imports, complejidad, tipos) no debería ser tema de review humano — para eso están linter, formatter y type checker en CI. El review humano se reserva para lo que la máquina no ve:

  1. Correctness / lógica de negocio — ¿el código hace lo que la historia pide? ¿edge cases, condiciones de carrera, off-by-one?
  2. Diseño / arquitectura — ¿está en la capa correcta? ¿introduce acoplamiento innecesario? ¿es la abstracción adecuada o es over-engineering?
  3. Seguridad — inputs sin validar, secrets hardcodeados, SQL sin parametrizar, authz faltante.
  4. Tests — ¿los tests prueban comportamiento (no implementación)? ¿cubren los caminos de error, no solo el happy path?
  5. Legibilidad y mantenibilidad — nombres, cohesión, ¿lo entenderá alguien en seis meses?

Cómo dar buen feedback. El feedback destructivo mata la práctica más rápido que cualquier bug. Reglas concretas de un review de calidad:

Ejemplo de checklist de proyecto (un artefacto que el senior crea, no improvisa):

## PR Review Checklist — <service>

### Correctness
- [ ] Behavior matches the acceptance criteria of the ticket
- [ ] Error paths handled (not only happy path); errors are typed/meaningful
- [ ] No obvious race conditions in async/concurrent code

### Design
- [ ] Logic lives in the right layer (router thin, service holds business logic)
- [ ] No duplicated logic that already exists in the codebase
- [ ] Public API / DB schema changes are backward compatible (or migration provided)

### Security
- [ ] No secrets in code/config committed
- [ ] All external input validated (Pydantic schema / query params)
- [ ] DB access parameterized; authz enforced on the endpoint

### Tests
- [ ] New logic has unit tests incl. failure cases
- [ ] Integration test for the wired path (API -> DB) when relevant
- [ ] Tests assert behavior, not implementation details

### Ops
- [ ] Meaningful logging at correct level; no PII in logs
- [ ] Migration is reversible / has a rollback plan

Integrarlo en CI (esto es lo que evalúan a nivel senior). El review humano y el review automático se combinan. La configuración concreta:

# .github/CODEOWNERS
# Reviewers required per path (auto-requested on PR)
/app/payments/      @payments-team @a-senior
/app/auth/          @security-team
/infra/             @platform-team
*.sql               @db-owners

Continuous Integration

CI significa que los miembros del equipo integran su trabajo con frecuencia sobre una base de código compartida, mergeando cambios al VCS y creando/probando builds automáticamente. La idea central: cada integración se verifica sola, y si algo se rompe se detecta de inmediato, no en release. A nivel senior el objetivo declarado es identificar e implementar mejoras en el proceso de CI: diseñarlo, analizarlo por cuellos de botella y optimizarlo.

Anatomía de un pipeline: stages, jobs, steps. Un pipeline se organiza en jobs que corren en runners aislados. Los jobs se encadenan por dependencias (needs), lo que crea etapas lógicas: primero checks baratos y rápidos (lint, types, security scan), luego tests (más caros), luego build del artifact (solo si lo anterior pasó). Poner los checks baratos primero es una optimización clave: fail fast — si el lint falla en 20 segundos, no gastás 8 minutos de tests.

# .github/workflows/ci.yml
name: CI

on:
  push:
    branches: [main]
  pull_request:
    branches: [main]

# Cancel superseded runs on the same PR to save runner minutes
concurrency:
  group: ci-${{ github.ref }}
  cancel-in-progress: true

jobs:
  # Stage 1: cheap static checks — fail fast
  quality:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
      - uses: actions/setup-python@v5
        with:
          python-version: "3.12"
          cache: pip                      # cache deps between runs
      - run: pip install -r requirements-dev.txt
      - name: Lint
        run: ruff check app/ tests/
      - name: Format check
        run: ruff format --check app/ tests/
      - name: Type check
        run: mypy app/
      - name: Security scan (SAST)
        run: bandit -r app/ -ll
      - name: Dependency audit
        run: pip-audit -r requirements.txt

  # Stage 2: tests — only runs if quality passed
  test:
    needs: quality
    runs-on: ubuntu-latest
    services:
      postgres:
        image: postgres:16-alpine
        env:
          POSTGRES_DB: test
          POSTGRES_USER: test
          POSTGRES_PASSWORD: test
        ports: ["5432:5432"]
        options: >-
          --health-cmd pg_isready --health-interval 10s
          --health-timeout 5s --health-retries 5
      redis:
        image: redis:7-alpine
        ports: ["6379:6379"]
    steps:
      - uses: actions/checkout@v4
      - uses: actions/setup-python@v5
        with:
          python-version: "3.12"
          cache: pip
      - run: pip install -r requirements.txt -r requirements-dev.txt
      - name: Run tests with coverage
        env:
          DATABASE_URL: postgresql://test:test@localhost:5432/test
          REDIS_URL: redis://localhost:6379/0
        run: pytest tests/ -v --cov=app --cov-report=xml --junitxml=report.xml
      - name: Coverage gate
        run: coverage report --fail-under=80
      - name: Upload test artifacts        # artifacts: reports outlive the runner
        if: always()                        # upload even when tests fail
        uses: actions/upload-artifact@v4
        with:
          name: test-reports
          path: |
            report.xml
            coverage.xml

  # Stage 3: build immutable artifact — only on main, only if tests passed
  build:
    needs: test
    if: github.ref == 'refs/heads/main'
    runs-on: ubuntu-latest
    permissions:
      contents: read
      packages: write
    steps:
      - uses: actions/checkout@v4
      - name: Build & push image tagged by commit SHA
        run: |
          IMAGE=ghcr.io/${{ github.repository }}:${{ github.sha }}
          docker build -t "$IMAGE" .
          docker push "$IMAGE"

Artifacts. Un artifact es cualquier salida del pipeline que necesita sobrevivir al runner efímero: reportes de test/coverage (para mostrarlos o hacer trending), binarios/wheels, y sobre todo la imagen de contenedor inmutable. Regla senior: el artifact que se testeó es exactamente el que se despliega — se etiqueta por commit SHA (no latest), se construye una sola vez, y se promueve entre entornos sin reconstruir. Reconstruir por entorno rompe la garantía de “lo que testeé es lo que corre”.

Quality gates. Un gate es una condición que bloquea el avance del pipeline (y por ende el merge, vía branch protection). Los gates típicos de un servicio Python:

GateHerramientaQué garantiza
Lintruff checkCódigo consistente, sin errores de estilo/lógica detectables estáticamente
Formatruff format --checkEstilo uniforme, cero diffs de formato
Typesmypy / pyrightErrores de tipo antes de runtime
Security (SAST)banditPatrones inseguros en el código
Dependenciaspip-audit / safetyCVEs conocidos en dependencias
TestspytestComportamiento correcto
Coveragecoverage --fail-underNo entra código sin tests

Optimización (nivel senior). Analizar el pipeline por cuellos de botella e implementar mejoras es exactamente el criterio A3. Palancas concretas:


Release Strategy

La release strategy (o release management) es el proceso de planificar y controlar cómo el software llega a sus entornos y usuarios: cuándo, cómo y a quién. A nivel senior cubre gestión de dependencias, estrategias de branching, setup de entornos, y las técnicas de deployment que acotan el riesgo de cada release.

Branching y su relación con la release. Dos familias:

Decisión senior típica para microservicios con CI/CD: trunk-based + feature flags, deploy automático del artifact que salió verde del pipeline.

Gestión de dependencias y release repository. Parte del temario senior. El problema: builds no reproducibles y “dependency hell”. Soluciones: pinnear con lockfile con hashes (pip-compile/uv lock/poetry.lock) para builds deterministas; y usar un repositorio de artifacts/paquetes (Artifactory, Nexus, un index privado, GHCR/ECR para imágenes) para cachear dependencias, hostear paquetes internos, y desacoplarse de la disponibilidad del index público. El artifact inmutable versionado por SHA vive en ese repositorio y es lo que se promueve entre entornos.

Entornos. Un senior “drivea” el setup de entornos production-like: al menos dev → staging (lo más parecido a prod posible) → prod, con configuración separada por entorno (env vars / secrets manager, nunca en la imagen) y el mismo artifact promovido a través de ellos. Esto conecta con Infrastructure as Code: los entornos se definen declarativamente (Terraform/Pulumi/manifests) para que sean reproducibles y versionados, no configurados a mano.

Técnicas de deployment (el corazón del tema). Cada una hace un trade-off distinto entre riesgo, costo de infraestructura y velocidad de rollback:

Blue-green. Dos entornos idénticos: blue (actual, sirviendo tráfico) y green (nueva versión). Se despliega y smoke-testea en green con cero tráfico real; luego el load balancer/router conmuta todo el tráfico de blue a green de golpe. Rollback = volver a apuntar a blue (instantáneo). Ventaja: cutover atómico, rollback trivial, blue queda caliente como red de seguridad. Costo: mantener dos entornos full (2x infra durante la transición) y manejar el estado compartido (migraciones de DB, sesiones) con cuidado.

        ┌─────────┐
        │   LB /  │
        │  router │
        └────┬────┘
   switch    │
  blue→green │
     ┌───────┴───────┐
     ▼               ▼
 ┌────────┐     ┌─────────┐
 │  BLUE  │     │  GREEN  │
 │ v1.4   │     │  v1.5   │  ← deploy + smoke test here first,
 │(live)  │     │(staged) │     then flip 100% of traffic
 └────────┘     └─────────┘

Canary. Se libera la nueva versión a un subconjunto pequeño de tráfico/usuarios (1% → 5% → 25% → 100%), observando métricas (error rate, latencia p95/p99, business KPIs) en cada paso. Si las métricas se degradan, se aborta y se revierte antes de afectar a todos. Ventaja: limita el blast radius, permite validar con tráfico real. Costo: requiere buen observability y routing por porcentaje/segmento (service mesh, ingress con weights), y la nueva y la vieja versión conviven, así que ambas deben ser compatibles con el mismo estado/DB.

 100% users

     ├──► 95%  ──► v1.4 (stable)
     └──►  5%  ──► v1.5 (canary)  ── watch p95, error rate ──►
                                    ok? ramp 5→25→50→100 | bad? roll back

Feature flags. Desacoplan deploy de release: el código de una feature se mergea y despliega apagado tras un flag, y se enciende (para todos, o para un cohorte) sin un nuevo deploy. Habilitan trunk-based (mergear trabajo incompleto sin exponerlo), A/B testing, kill-switches, y rollouts graduales a nivel de feature en vez de a nivel de deploy. El costo: los flags son deuda técnica si no se limpian; hay que tener un proceso para retirarlos.

# Feature flag guarding a new code path — deploy dark, enable later
from app.flags import flags

async def get_price(order: Order, user: User) -> Decimal:
    if flags.is_enabled("new_pricing_engine", user_id=user.id):
        return await new_pricing_engine.quote(order)   # released to a cohort
    return await legacy_pricing.quote(order)            # default / fallback

Rollback. La pregunta senior no es “¿cómo despliego?” sino “¿cómo revierto en 30 segundos cuando algo sale mal?”. Cada técnica tiene su rollback: blue-green → reapuntar el router; canary → cortar el ramp y bajar el peso a 0; feature flag → apagar el flag (el rollback más rápido, sin redeploy). El punto crítico y más difícil: las migraciones de base de datos no se revierten como el código. Regla de oro: hacer cambios de schema backward-compatible (expand/contract): primero expandir (agregar columna nullable, escribir en vieja y nueva), desplegar el código que las usa, y solo después contraer (borrar lo viejo) en un release posterior. Así el rollback del código no deja la DB en un estado incompatible.


Code Standards

Los coding standards son un conjunto de guías —organización de archivos, indentación, comentarios, convenciones de nombres, prácticas y principios— que mejoran legibilidad y mantenibilidad. Clave: no los impone el compilador; sin tooling que los haga cumplir, son buenas intenciones. A nivel senior el criterio es “saber diseñar e implementar los estándares”: convertirlos de documento a gates automáticos.

Linters y formatters. La distinción importa:

# pyproject.toml — single source of truth for standards
[tool.ruff]
line-length = 100
target-version = "py312"

[tool.ruff.lint]
# E/W pycodestyle, F pyflakes, I import sort, N naming,
# UP pyupgrade, B bugbear, S bandit-style security, C4 comprehensions
select = ["E", "W", "F", "I", "N", "UP", "B", "S", "C4"]
ignore = ["E501"]          # line length handled by the formatter
[tool.ruff.lint.per-file-ignores]
"tests/*" = ["S101"]       # allow assert in tests

[tool.mypy]
python_version = "3.12"
strict = true              # the senior default: opt into all strict checks
warn_return_any = true
disallow_untyped_defs = true
plugins = ["pydantic.mypy"]

[tool.pytest.ini_options]
addopts = "-ra --strict-markers --strict-config"
testpaths = ["tests"]

Type checking. En Python las type hints (PEP 484) son opcionales en runtime pero un type checker estático (mypy, pyright) las verifica antes de ejecutar, atrapando una clase entera de bugs (None donde no debía, firma incompatible, atributo inexistente). El estándar senior es activar strict = true y prohibir funciones sin anotar (disallow_untyped_defs), idealmente incremental en un codebase legacy (empezar por módulos nuevos). Los type hints además son documentación viva y potencian el autocompletado y el refactor.

Pre-commit hooks. Corren los checks antes de que el commit exista, dando feedback en segundos en la máquina del dev en vez de esperar 5 minutos de CI. El framework pre-commit los gestiona de forma declarativa y versionada:

# .pre-commit-config.yaml
repos:
  - repo: https://github.com/astral-sh/ruff-pre-commit
    rev: v0.6.0
    hooks:
      - id: ruff            # lint, autofix
        args: [--fix]
      - id: ruff-format     # format
  - repo: https://github.com/pre-commit/mirrors-mypy
    rev: v1.11.0
    hooks:
      - id: mypy
        additional_dependencies: [pydantic]
  - repo: https://github.com/pre-commit/pre-commit-hooks
    rev: v4.6.0
    hooks:
      - id: trailing-whitespace
      - id: end-of-file-fixer
      - id: check-added-large-files
      - id: detect-private-key   # block committing secrets

Principio de defensa en profundidad: las mismas reglas corren en tres capas — pre-commit (local, rápido, opcional de saltear con --no-verify), CI (bloqueante, no salteable), y editor (feedback en vivo). Pre-commit reduce el ida-y-vuelta con CI, pero CI es la autoridad porque no se puede saltear.

Métricas de calidad de código. Un senior mide, no opina:


Ejercicios

1. Diseñar un checklist de code review para un servicio de pagos. Escribí un checklist de PR específico para un microservicio que procesa pagos (maneja dinero, idempotencia y datos sensibles), con al menos 4 categorías. Justificá por qué cada categoría existe.

Solución

Lo importante es que el checklist sea específico del dominio, no genérico. Categorías y por qué:

## PR Review — payment-service

### Money correctness (dominio crítico)
- [ ] Amounts use Decimal, never float (float pierde precisión en dinero)
- [ ] Currency is explicit and validated; no mixing currencies
- [ ] Rounding rule is defined and consistent (half-up documented)

### Idempotency & concurrency
- [ ] Write operations accept and honor an Idempotency-Key
- [ ] Retries can't double-charge (dedup at the DB with a unique constraint)
- [ ] No race between "check balance" and "debit" (transaction / row lock)

### Security & compliance
- [ ] No PAN / CVV logged or persisted in plaintext
- [ ] Secrets (gateway keys) from secrets manager, not code/env-in-repo
- [ ] Endpoint enforces authz (only the owner/service can move funds)

### Observability & recovery
- [ ] Every state transition is auditable (event/log with correlation id)
- [ ] Failure path leaves the system in a recoverable state (no partial debit)
- [ ] DB migration is backward-compatible + has a rollback plan

El punto senior: el checklist codifica los riesgos de este dominio. Un checklist genérico (“¿hay tests? ¿nombres claros?”) no habría atrapado el double-charge en retry, que es el bug caro real acá.

2. Detectar y arreglar un cuello de botella de CI. Un pipeline tarda 22 minutos: install deps (6 min) corre en cada uno de 3 jobs, los tests corren serialmente (9 min), y el lint corre después de los tests. Cada PR reejecuta todo aunque se abandone. Proponé cambios concretos.

Solución

Problemas y fixes:

  1. Deps reinstaladas 3× sin cache → activar cache de pip (cache: pip en setup-python) o cachear el venv/~/.cache/pip. Ahorra la mayor parte de esos 6 min en runs subsecuentes.
  2. Lint después de tests → reordenar: lint/format/types/security primero (segundos), tests después con needs. Fail fast: un error de lint mata el run en < 1 min en vez de 9.
  3. Tests serializadospytest -n auto (pytest-xdist) para paralelizar en los cores del runner; separar E2E/load a un job aparte que corra solo en main o nightly.
  4. Runs obsoletosconcurrency: { group: ci-${{ github.ref }}, cancel-in-progress: true } cancela ejecuciones viejas cuando se pushea de nuevo al mismo PR.

Resultado esperado: fail-fast en < 1 min para errores triviales, y camino verde reducido de ~22 min a un rango mucho menor por caching + paralelización. El principio: medir dónde se va el tiempo y atacar el mayor costo primero (deps y serialización).

3. Diseñar el rollout de un cambio con migración de DB. Necesitás renombrar la columna orders.amount a orders.total_amount en un servicio con deploy continuo y sin downtime. Describí la secuencia usando expand/contract.

Solución

Un rename directo (ALTER ... RENAME COLUMN) rompe: durante el rollout conviven código viejo (lee amount) y nuevo (lee total_amount), y un rollback dejaría la DB incompatible. Secuencia expand/contract en releases separados:

  1. Expand — migración que agrega total_amount (nullable), backfill de datos, y un trigger o doble escritura para mantener ambas columnas sincronizadas. Sin cambio de código todavía. Rollback trivial: la columna nueva se ignora.
  2. Migrate code — desplegar código que lee/escribe total_amount pero tolera ambas. Ahora el tráfico usa la columna nueva. Si algo falla, rollback al código viejo sigue funcionando porque amount se mantuvo sincronizado.
  3. Contract — una vez que la versión nueva está estable en prod y no hay riesgo de rollback, migración final que quita la doble escritura y dropea amount.

Cada paso es desplegable y reversible por sí solo. La clave senior: nunca acoplar un cambio de schema destructivo al mismo deploy que el código que lo necesita.

4. Configurar los estándares de un proyecto nuevo desde cero. Un equipo arranca un servicio FastAPI sin estándares. Definí la configuración mínima que haga cumplir formato, lint, tipos y seguridad de forma automática en las tres capas (editor, local, CI).

Solución
  • Fuente única de verdad: todo en pyproject.toml (config de ruff con select incluyendo S para seguridad, y mypy con strict = true). Un solo lugar que editor, pre-commit y CI leen.
  • Editor: extensión de Ruff + mypy/pyright para feedback en vivo; settings.json del repo con format-on-save. Reduce fricción antes de commitear.
  • Local (pre-commit): .pre-commit-config.yaml con ruff (fix + format), mypy, detect-private-key, check-added-large-files. pre-commit install en el onboarding. Rápido, atrapa el 90% antes de push.
  • CI (autoridad, bloqueante): job quality con ruff check, ruff format --check, mypy app/, bandit -r app/, pip-audit, como checks requeridos en branch protection. No salteable con --no-verify.

El punto: las mismas reglas en tres capas (defensa en profundidad), una sola config, y CI como autoridad no salteable. Sin el gate de CI, los estándares son opcionales y se erosionan.

5. (Discusión) Post-commit review + trunk-based, ¿cuándo sí? Un lead propone pasar de pre-merge review estricto a commit-then-review con trunk-based para acelerar. ¿Qué condiciones deben cumplirse para que sea seguro, y qué riesgos introduce?

Solución

Condiciones necesarias para que sea seguro:

  • CI fuerte y confiable: suite de tests con buena cobertura de comportamiento + gates (lint, types, security) bloqueantes, porque el código entra a main antes del review humano. El CI es la primera línea de defensa.
  • Equipo maduro y de alto trust: devs con criterio para no romper main; commits chicos y frecuentes.
  • Feature flags: para mergear trabajo incompleto sin exponerlo.
  • Deploy reversible rápido (flags / rollback de artifact) por si algo defectuoso llega a prod.
  • Cultura de review post-merge real: el review sigue ocurriendo, solo que después; si nadie revisa después de mergear, se pierde la transferencia de conocimiento y la detección de defectos de diseño.

Riesgos: código defectuoso ya en main puede bloquear a otros; si la disciplina baja, el review post-merge se saltea de facto. Trade-off: gana velocidad de integración a cambio de depender por completo de la automatización y la madurez del equipo. Para un equipo nuevo o un dominio de alto riesgo (pagos, salud), pre-merge estricto sigue siendo la elección correcta.


Preguntas tipo entrevista (EN)

Q1: What should a human reviewer focus on, given that CI already runs linters, formatters, and a type checker?

Q2: A teammate opens a 900-line PR and pings you for review. What’s the senior response?

Q3: What’s the difference between blue-green and canary deployments, and when would you pick each?

Q4: How do feature flags relate to deployment, and what’s their cost?

Q5: Why can’t you just git revert the code when a bad release included a database migration?

Q6: Your team argues about code style in every PR. How do you fix it at the root?

Q7: Is 100% test coverage a good goal to enforce as a gate?


Referencias