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:
- Correctness / lógica de negocio — ¿el código hace lo que la historia pide? ¿edge cases, condiciones de carrera, off-by-one?
- Diseño / arquitectura — ¿está en la capa correcta? ¿introduce acoplamiento innecesario? ¿es la abstracción adecuada o es over-engineering?
- Seguridad — inputs sin validar, secrets hardcodeados, SQL sin parametrizar, authz faltante.
- Tests — ¿los tests prueban comportamiento (no implementación)? ¿cubren los caminos de error, no solo el happy path?
- 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:
- Ataca el código, no a la persona. “Esta función tiene tres responsabilidades” en vez de “hiciste esto mal”.
- Distinguí bloqueante de preferencia. Un convención útil son prefijos etiquetados:
blocking:(hay que arreglarlo para mergear),question:(no entiendo, explicame),nit:(preferencia menor, opcional),praise:(esto está bien resuelto). Sin esta distinción, unnit:de naming frena un PR correcto durante días. - Justifica con el porqué, idealmente con referencia (un principio, un doc, un benchmark). “Esto es O(n²) sobre una lista que en prod tiene 10k items” pesa más que “esto es lento”.
- Preferí sugerencias sobre órdenes. GitHub/GitLab tienen suggested changes aplicables con un click para nits triviales.
- Timebox y tamaño. Un PR de más de ~400 líneas pierde efectividad de detección de defectos drásticamente; la recomendación senior es PRs chicos y frecuentes. Y el review no debería tardar días: acordar un SLA (p. ej. revisar en < 1 día hábil) es parte de establecer la práctica.
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:
- Branch protection / merge requirements. En la rama protegida, exigir: N aprobaciones, que el status check de CI esté verde, que la rama esté al día con main, y opcionalmente aprobación de un code owner.
- CODEOWNERS. Un archivo que rutea automáticamente el review a los responsables de cada path — esto implementa la “jerarquía de reviewers” que pide el nivel senior.
# .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
- Automatizar lo mecánico para que el humano no lo revise: linter/formatter/type-check/security-scan corren como checks obligatorios; herramientas como
pre-commit.ci,danger, o bots de review reducen el ruido. El principio: si una regla se puede verificar mecánicamente, no gastes tiempo humano en ella.
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:
| Gate | Herramienta | Qué garantiza |
|---|---|---|
| Lint | ruff check | Código consistente, sin errores de estilo/lógica detectables estáticamente |
| Format | ruff format --check | Estilo uniforme, cero diffs de formato |
| Types | mypy / pyright | Errores de tipo antes de runtime |
| Security (SAST) | bandit | Patrones inseguros en el código |
| Dependencias | pip-audit / safety | CVEs conocidos en dependencias |
| Tests | pytest | Comportamiento correcto |
| Coverage | coverage --fail-under | No 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:
- Caching de dependencias (
cache: pip, cache deuv, cache de layers Docker con BuildKit) — el mayor ahorro en pipelines Python. - Paralelización: jobs independientes en paralelo;
pytest -n auto(pytest-xdist) para distribuir tests. - Fail fast + concurrency cancel: cancelar runs obsoletos y ordenar de barato a caro.
- Selección de tests / matrices: correr suites pesadas (E2E, load) solo en main o nightly, no en cada PR.
- Medir: el tiempo de pipeline es una métrica de producto interno; si un PR tarda 25 min en verde, la gente deja de integrar seguido y CI pierde su razón de ser.
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:
- Trunk-based: una rama principal (
main) siempre desplegable, feature branches muy cortas (< 1–2 días) que mergean seguido. Encaja con CI/CD real y deploy continuo. Complejidad baja, feedback rápido, pero exige feature flags para esconder trabajo incompleto y una suite de tests fuerte. - GitFlow:
main,develop, y ramasfeature/release/hotfix. Merges infrecuentes, releases programados. Complejidad alta; útil para software versionado con soporte de múltiples versiones o releases calendarizados, pero introduce fricción de merge y retrasa la integración.
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:
- Formatter (
ruff format, black): reescribe el código a un estilo canónico (largo de línea, comillas, saltos). No opina sobre lógica; elimina de raíz las discusiones de estilo y los diffs de formato. Determinista. - Linter (
ruff check, flake8, pylint): detecta problemas más allá del formato — imports sin usar, variables sin usar, code smells, antipatrones, complejidad ciclomática alta, posibles bugs. En el ecosistema Python actual, Ruff consolida linter + formatter + import-sorting en una sola herramienta rapidísima escrita en Rust, reemplazando a flake8/isort/black.
# 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:
- Coverage — porcentaje de código ejecutado por los tests. Útil como gate (
--fail-under=80) y para detectar zonas sin tests, pero es un piso, no un techo: 100% de coverage no significa tests buenos (podés cubrir una línea sin assertear nada). Distinguir line vs branch coverage. - Complejidad ciclomática — número de caminos independientes en una función; alta complejidad correlaciona con bugs y dificultad de test (
ruffcon reglasC901, oradon). - Maintainability index / duplicación / code smells — herramientas como SonarQube agregan estas métricas, marcan hotspots y trackean deuda técnica y security hotspots a lo largo del tiempo, integrándose al pipeline como un gate más.
- La trampa: convertir una métrica en objetivo la corrompe (ley de Goodhart). El coverage al 80% es un piso razonable; forzar 100% incentiva tests basura. Las métricas informan decisiones, no las reemplazan.
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:
- Deps reinstaladas 3× sin cache → activar cache de pip (
cache: pipensetup-python) o cachear el venv/~/.cache/pip. Ahorra la mayor parte de esos 6 min en runs subsecuentes. - 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. - Tests serializados →
pytest -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. - Runs obsoletos →
concurrency: { 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:
- 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. - Migrate code — desplegar código que lee/escribe
total_amountpero tolera ambas. Ahora el tráfico usa la columna nueva. Si algo falla, rollback al código viejo sigue funcionando porqueamountse mantuvo sincronizado. - 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 deruffconselectincluyendoSpara seguridad, ymypyconstrict = true). Un solo lugar que editor, pre-commit y CI leen. - Editor: extensión de Ruff + mypy/pyright para feedback en vivo;
settings.jsondel repo con format-on-save. Reduce fricción antes de commitear. - Local (pre-commit):
.pre-commit-config.yamlcon ruff (fix + format), mypy,detect-private-key,check-added-large-files.pre-commit installen el onboarding. Rápido, atrapa el 90% antes de push. - CI (autoridad, bloqueante): job
qualityconruff 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?
- ❌ Wrong / trap: “Check the style, formatting, naming, and unused imports carefully.” — Fails because it spends scarce human attention on exactly what the machine already enforces deterministically. If your review comments are mostly formatting nits, your tooling is misconfigured, not your reviewers.
- ✅ Correct: The human focuses on what tools can’t see: business-logic correctness, edge cases and race conditions, design/architecture fit (right layer, right abstraction, no needless coupling), security issues (missing authz, unvalidated input), and whether tests assert behavior and cover failure paths.
- ⭐ Optimal (senior): Establish the split explicitly: everything mechanically checkable becomes a blocking CI gate (ruff/mypy/bandit), so review time is reserved for correctness, design, and knowledge transfer. Add CODEOWNERS to route reviews by expertise, label comments (
blocking:/nit:/question:) to separate must-fix from preference, and keep PRs small (<~400 lines) because defect-detection efficiency collapses on large diffs.
Q2: A teammate opens a 900-line PR and pings you for review. What’s the senior response?
- ❌ Wrong / trap: “Read it top to bottom and approve if nothing jumps out.” — Fails: research and practice show review effectiveness drops sharply past a few hundred lines — reviewers skim, defect detection plummets, and a rubber-stamp approval gives false confidence.
- ✅ Correct: Push back on size: ask to split it into smaller, independently reviewable PRs (e.g., refactor separate from feature, one concern per PR). If it truly can’t be split, review it in focused passes (design first, then correctness, then tests) and timebox.
- ⭐ Optimal (senior): Treat PR size as a process problem, not a one-off. Establish norms: small PRs, feature flags so incomplete work can merge behind a flag, and a review SLA. Large PRs are usually a symptom of long-lived branches — the fix is trunk-based flow with frequent integration, which is also what makes CI meaningful.
Q3: What’s the difference between blue-green and canary deployments, and when would you pick each?
- ❌ Wrong / trap: “They’re the same thing — both deploy a new version safely.” — Fails: it conflates two distinct risk/traffic models and shows no grasp of blast radius or rollback mechanics.
- ✅ Correct: Blue-green keeps two full environments and switches 100% of traffic at once from old (blue) to new (green) after smoke-testing green; rollback is repointing the router. Canary releases the new version to a small slice of traffic (1%→5%→…) while watching metrics, ramping up only if healthy; rollback is dropping the weight to 0. Blue-green gives atomic cutover; canary limits blast radius and validates with real traffic.
- ⭐ Optimal (senior): Choose by risk tolerance and infra. Canary when you have solid observability (error rate, p95/p99, business KPIs) and weighted routing (service mesh/ingress) and want to catch issues real users would hit before everyone hits them. Blue-green when you want the simplest atomic switch and can afford 2x infra during transition. Both require the two versions to be compatible with shared state — which is why DB changes must be backward-compatible (expand/contract), independent of the traffic-shifting technique.
Q4: How do feature flags relate to deployment, and what’s their cost?
- ❌ Wrong / trap: “Feature flags are just if-statements to turn features on and off, basically free.” — Fails: it misses that flags decouple deploy from release (the whole point) and ignores that flags are real technical debt with a lifecycle.
- ✅ Correct: Feature flags decouple deploy from release: code ships to production turned off, then is enabled (for everyone or a cohort) with no new deploy. This enables trunk-based development (merge incomplete work dark), gradual rollouts, A/B tests, and instant kill-switches — the fastest rollback there is (flip the flag, no redeploy).
- ⭐ Optimal (senior): The cost is lifecycle management: every flag is a branch in code and config that adds complexity and testing surface, and stale flags rot. You need a process to remove flags once a feature is fully rolled out, ownership per flag, and awareness that flags multiply the state space your tests must cover. Used with discipline they’re powerful; left uncleaned they become permanent conditional debt.
Q5: Why can’t you just git revert the code when a bad release included a database migration?
- ❌ Wrong / trap: “Revert the commit and redeploy — that undoes everything.” — Fails: reverting code doesn’t undo a schema change. If the migration dropped or renamed a column, the reverted (old) code now runs against a schema it doesn’t understand, and the app breaks harder than before.
- ✅ Correct: Code and schema roll back differently. The safe approach is to make schema changes backward-compatible so a code rollback stays valid: use expand/contract — add the new column, dual-write, migrate the code, and only drop the old column in a later release once rollback risk is gone.
- ⭐ Optimal (senior): Never couple a destructive schema change to the same deploy as the code that needs it. Sequence it across releases (expand → migrate code → contract), keep each step independently deployable and reversible, and pair with a fast code-level rollback (feature flag / repoint to previous artifact). This is what actually makes “zero-downtime, reversible deploys” true rather than aspirational.
Q6: Your team argues about code style in every PR. How do you fix it at the root?
- ❌ Wrong / trap: “Write a coding standards document everyone agrees to follow.” — Fails: a document nobody’s tooling enforces is just habitual practice — ‘coding standards are not enforced by compilers.’ It’ll be ignored under deadline pressure and re-litigated every PR.
- ✅ Correct: Automate it. A formatter (ruff format/black) makes style non-negotiable and deterministic, and a linter (ruff) catches smells — both as CI gates so nothing merges that violates them. Style stops being a discussion because the machine decides.
- ⭐ Optimal (senior): Enforce in depth: one source of truth (
pyproject.toml), the same rules in the editor (format-on-save), pre-commit (fast local feedback), and CI (the non-skippable authority). Add type checking (mypy --strict) and a security linter (bandit) to the same gates. Now review comments are about design and correctness, never brace placement — which is the real payoff.
Q7: Is 100% test coverage a good goal to enforce as a gate?
- ❌ Wrong / trap: “Yes — enforce 100% coverage so all code is tested and bug-free.” — Fails: coverage measures which lines executed during tests, not whether they were meaningfully asserted. You can hit 100% with tests that assert nothing, and chasing it incentivizes junk tests. High coverage ≠ correct code.
- ✅ Correct: Coverage is a useful floor, not a target — a gate like
--fail-under=80catches wholly untested areas, and branch coverage is more informative than line coverage. But it’s a proxy: the goal is tests that assert behavior and cover failure paths, which coverage alone can’t measure. - ⭐ Optimal (senior): Use coverage as one signal among several (with cyclomatic complexity, mutation testing for assertion quality, and SonarQube-style trends/hotspots), and beware Goodhart’s law — the moment a metric becomes the target it stops being a good metric. Set a pragmatic floor, focus review on whether the important behaviors and edge cases are actually tested, and reserve strict thresholds for critical modules (e.g., payments) rather than blanket-enforcing 100%.
Referencias
- Google Engineering Practices — Code Review: https://google.github.io/eng-practices/review/
- Conventional Comments (labeled review feedback): https://conventionalcomments.org/
- Martin Fowler — Continuous Integration: https://martinfowler.com/articles/continuousIntegration.html
- Martin Fowler — Blue-Green Deployment: https://martinfowler.com/bliki/BlueGreenDeployment.html
- Martin Fowler — Canary Release: https://martinfowler.com/bliki/CanaryRelease.html
- Martin Fowler — Feature Toggles (Flags): https://martinfowler.com/articles/feature-toggles.html
- Trunk Based Development: https://trunkbaseddevelopment.com/
- Ruff (linter + formatter): https://docs.astral.sh/ruff/
- mypy documentation: https://mypy.readthedocs.io/
- pre-commit framework: https://pre-commit.com/
- Bandit (Python SAST): https://bandit.readthedocs.io/
- GitHub Actions — workflow syntax & artifacts: https://docs.github.com/actions
- Refactoring Databases — expand/contract / evolutionary DB design: https://martinfowler.com/articles/evodb.html