8 min lectura • Guide 111 of 877
Manteniendo Estándares Consistentes de Code Review
Los estándares de code review eliminan debates subjetivos definiendo qué significa "suficientemente bueno" para tu equipo. Las integraciones de GitHub, GitLab, y Bitbucket de GitScrum muestran actividad de pull requests junto al tracking de tareas, mientras documentación NoteVault y checklists de revisión ayudan a equipos a mantener expectativas consistentes, reducir tiempo de revisión, y convertir code review en oportunidad de aprendizaje en lugar de cuello de botella.
Estableciendo Estándares
Framework Criterios Review
DIMENSIONES CODE REVIEW:
┌─────────────────────────────────────────────────────────────┐
│ QUÉ REVISAR │
├─────────────────────────────────────────────────────────────┤
│ │
│ TIER 1: DEBE PASAR (Bloqueante) │
│ ┌─────────────────────────────────────────────────────────┐│
│ │ ☐ Funciona correctamente (implementa requerimientos) ││
│ │ ☐ Sin bugs obvios o casos borde ││
│ │ ☐ Tests pasan (unit, integración) ││
│ │ ☐ Sin vulnerabilidades seguridad introducidas ││
│ │ ☐ Sin cambios breaking sin path migración ││
│ │ ☐ Sin datos sensibles expuestos (keys, passwords) ││
│ │ ││
│ │ Estado: PR no puede mergear hasta que todo checkeado ││
│ └─────────────────────────────────────────────────────────┘│
│ │
│ TIER 2: DEBERÍA PASAR (Alta Prioridad) │
│ ┌─────────────────────────────────────────────────────────┐│
│ │ ☐ Código es legible y entendible ││
│ │ ☐ Sigue convenciones naming del equipo ││
│ │ ☐ Manejo errores es apropiado ││
│ │ ☐ Performance es aceptable ││
│ │ ☐ Tiene cobertura tests suficiente ││
│ │ ☐ Documentación actualizada si necesario ││
│ │ ││
│ │ Estado: Abordar antes de merge, o crear tarea follow-up ││
│ └─────────────────────────────────────────────────────────┘│
│ │
│ TIER 3: BUENO TENER (Sugerencias) │
│ ┌─────────────────────────────────────────────────────────┐│
│ │ ☐ Podría ser más elegante/conciso ││
│ │ ☐ Preferencias menores de estilo ││
│ │ ☐ Enfoques alternativos a considerar ││
│ │ ☐ Oportunidades refactoring para después ││
│ │ ││
│ │ Estado: No-bloqueante, autor decide si abordar ││
│ └─────────────────────────────────────────────────────────┘│
│ │
│ DOCUMENTAR EN NOTEVAULT: │
│ ┌─────────────────────────────────────────────────────────┐│
│ │ Crear documento "Estándares Code Review" para equipo ││
│ │ Incluir ejemplos para cada tier ││
│ │ Referenciar en template PR ││
│ │ Revisar trimestralmente para actualizaciones ││
│ └─────────────────────────────────────────────────────────┘│
│ │
└─────────────────────────────────────────────────────────────┘
Convención Comentarios
TIPOS COMENTARIOS REVIEW:
┌─────────────────────────────────────────────────────────────┐
│ PREFIJOS COMENTARIOS CLAROS │
├─────────────────────────────────────────────────────────────┤
│ │
│ BLOQUEANTE: │
│ ┌─────────────────────────────────────────────────────────┐│
│ │ [BLOQUEANTE] Esto causará null pointer en producción ││
│ │ ││
│ │ Significado: PR no puede mergear hasta abordar ││
│ │ Responsabilidad revisor: Explicar por qué es bloqueante ││
│ │ Respuesta autor: Debe arreglar o discutir ││
│ └─────────────────────────────────────────────────────────┘│
│ │
│ PREGUNTA: │
│ ┌─────────────────────────────────────────────────────────┐│
│ │ [PREGUNTA] ¿Por qué estamos cacheando esto 24 horas? ││
│ │ ││
│ │ Significado: Necesito entender antes de aprobar ││
│ │ Responsabilidad revisor: Curiosidad genuina, no sarcasm ││
│ │ Respuesta autor: Explicar razonamiento ││
│ └─────────────────────────────────────────────────────────┘│
│ │
│ SUGERENCIA: │
│ ┌─────────────────────────────────────────────────────────┐│
│ │ [SUGERENCIA] Considera usar Optional aquí en su lugar ││
│ │ ││
│ │ Significado: Idea mejora no-bloqueante ││
│ │ Responsabilidad revisor: Proponer alternativa ││
│ │ Respuesta autor: Tómalo o déjalo, sin discusión req. ││
│ └─────────────────────────────────────────────────────────┘│
│ │
│ NIT: │
│ ┌─────────────────────────────────────────────────────────┐│
│ │ [NIT] Typo en nombre variable: "recieve" → "receive" ││
│ │ ││
│ │ Significado: Trivial, arreglar si fácil ││
│ │ Responsabilidad revisor: Mantener estos mínimos ││
│ │ Respuesta autor: Fix rápido o ignorar ││
│ └─────────────────────────────────────────────────────────┘│
│ │
│ ELOGIO: │
│ ┌─────────────────────────────────────────────────────────┐│
│ │ [BIEN] Solución limpia, aprendí algo aquí ││
│ │ ││
│ │ Significado: Feedback positivo, no solo crítica ││
│ │ Responsabilidad revisor: Ser genuino, no condescendient ││
│ │ Respuesta autor: Ninguna necesaria, se siente bien ││
│ └─────────────────────────────────────────────────────────┘│
│ │
└─────────────────────────────────────────────────────────────┘
Integración GitScrum
Trackeando Reviews
CONECTANDO PR A TAREAS:
┌─────────────────────────────────────────────────────────────┐
│ VISIBILIDAD EN GITSCRUM │
├─────────────────────────────────────────────────────────────┤
│ │
│ INTEGRACIÓN GITHUB/GITLAB/BITBUCKET: │
│ ┌─────────────────────────────────────────────────────────┐│
│ │ 1. Developer referencia tarea en PR/commit ││
│ │ "Fix timeout login usuario [GS-1234]" ││
│ │ ││
│ │ 2. Actividad PR se muestra en tarea GitScrum ││
│ │ Tarea #1234 muestra: ││
│ │ • PR abierto ││
│ │ • Reviews solicitados ││
│ │ • Comentarios agregados ││
│ │ • Estado merge ││
│ │ ││
│ │ 3. Equipo ve contexto completo sin cambiar herramientas ││
│ └─────────────────────────────────────────────────────────┘│
│ │
│ AUTOMATIZACIÓN COLUMNA WORKFLOW: │
│ ┌─────────────────────────────────────────────────────────┐│
│ │ Considera workflow: ││
│ │ ││
│ │ En Dev → En Review → Listo QA → Listo ││
│ │ ││
│ │ Columna "En Review" = PR abierto, esperando aprobación ││
│ │ ││
│ │ Mover a En Review cuando: ││
│ │ • PR creado y listo para review ││
│ │ • No todavía WIP (draft PR) ││
│ │ ││
│ │ Mover a Listo QA cuando: ││
│ │ • PR aprobado y mergeado ││
│ │ • Deployado a ambiente test ││
│ └─────────────────────────────────────────────────────────┘│
│ │
└─────────────────────────────────────────────────────────────┘
Eficiencia Review
Dimensionando PRs Correctamente
GUÍAS TAMAÑO PR:
┌─────────────────────────────────────────────────────────────┐
│ PRs PEQUEÑOS = MEJORES REVIEWS │
├─────────────────────────────────────────────────────────────┤
│ │
│ UMBRALES TAMAÑO: │
│ ┌─────────────────────────────────────────────────────────┐│
│ │ Líneas Cambiadas │ Calidad Review │ Acción ││
│ │ ─────────────────┼────────────────┼───────────────── ││
│ │ < 200 líneas │ ✅ Exhaustivo │ Tamaño ideal ││
│ │ 200-400 líneas │ ⚠️ Adecuado │ Aceptable ││
│ │ 400-800 líneas │ ❌ Apresurado │ Considerar dividir ││
│ │ > 800 líneas │ ❌ Rubber stamp │ Debe dividir ││
│ │ ││
│ │ Excepción: Refactors grandes con patrones claros ││
│ │ Excepción: Código generado o archivos config ││
│ └─────────────────────────────────────────────────────────┘│
│ │
│ ESTRATEGIAS DIVISIÓN: │
│ ┌─────────────────────────────────────────────────────────┐│
│ │ Feature muy grande para un PR: ││
│ │ ││
│ │ Opción 1: Slicing vertical ││
│ │ ┌─────────────────────────────────────────────────────┐ ││
│ │ │ PR 1: Happy path básico (slice delgado end-to-end) │ ││
│ │ │ PR 2: Manejo errores │ ││
│ │ │ PR 3: Casos borde │ ││
│ │ │ PR 4: Optimización performance │ ││
│ │ └─────────────────────────────────────────────────────┘ ││
│ │ ││
│ │ Opción 2: Capa por capa ││
│ │ ┌─────────────────────────────────────────────────────┐ ││
│ │ │ PR 1: Schema database + migraciones │ ││
│ │ │ PR 2: Endpoints API backend │ ││
│ │ │ PR 3: UI frontend │ ││
│ │ │ PR 4: Integración y tests │ ││
│ │ └─────────────────────────────────────────────────────┘ ││
│ │ ││
│ │ Trackear en GitScrum: ││
│ │ Tarea principal con subtareas para cada PR ││
│ │ Vincular todos PRs a tarea padre para visibilidad ││
│ └─────────────────────────────────────────────────────────┘│
│ │
└─────────────────────────────────────────────────────────────┘
Manejando Desacuerdos
Resolviendo Conflictos
CUANDO REVISOR Y AUTOR NO ESTÁN DE ACUERDO:
┌─────────────────────────────────────────────────────────────┐
│ RESOLUCIÓN CONFLICTOS CONSTRUCTIVA │
├─────────────────────────────────────────────────────────────┤
│ │
│ FRAMEWORK DECISIÓN: │
│ ┌─────────────────────────────────────────────────────────┐│
│ │ ││
│ │ ┌────────────────────────────────────────────────────┐ ││
│ │ │ ¿Es objetivamente malo (bug, seguridad, rompe)? │ ││
│ │ └──────────────────┬─────────────────────────────────┘ ││
│ │ │ ││
│ │ ┌─────────┴─────────┐ ││
│ │ Sí No ││
│ │ │ │ ││
│ │ ▼ ▼ ││
│ │ ┌─────────────────┐ ┌──────────────────────────────┐ ││
│ │ │ Debe arreglar │ │ ¿Hay estándar de equipo? │ ││
│ │ │ (Gana revisor) │ └───────────┬──────────────────┘ ││
│ │ └─────────────────┘ │ ││
│ │ ┌─────────┴─────────┐ ││
│ │ Sí No ││
│ │ │ │ ││
│ │ ▼ ▼ ││
│ │ ┌─────────────────┐ ┌────────────────┐ ││
│ │ │ Seguir estándar │ │ Autor decide │ ││
│ │ │ (Nadie gana, │ │ (Gana autor, │ ││
│ │ │ gana equipo) │ │ es su código) │ ││
│ │ └─────────────────┘ └────────────────┘ ││
│ │ ││
│ └─────────────────────────────────────────────────────────┘│
│ │
│ PATH ESCALACIÓN: │
│ ┌─────────────────────────────────────────────────────────┐│
│ │ Si no pueden acordar después de 2 rondas: ││
│ │ ││
│ │ 1. Limitar tiempo discusión (15 min call máximo) ││
│ │ 2. Si todavía sin acuerdo → traer tercera persona ││
│ │ 3. Tercera persona decide, decisión es final ││
│ │ ││
│ │ Nunca: Bloquear PR indefinidamente por pref. estilo ││
│ │ Nunca: Hacerlo personal ("siempre haces X") ││
│ │ Siempre: Enfocarse en código, no en coder ││
│ └─────────────────────────────────────────────────────────┘│
│ │
└─────────────────────────────────────────────────────────────┘