Code Review con PRs de GitHub
Los Pull Requests (PRs) son la puerta de integración para los repositorios de módulos Go.
//
Busca en todas las páginas de la documentación
Los Pull Requests (PRs) son la puerta de integración para los repositorios de módulos Go.
Una revisión efectiva mantiene los cambios pequeños, señala el impacto en go.mod y verifica las pruebas, no solo opiniones de estilo.
Un buen PR de Go declara el cambio de comportamiento, el impacto en el módulo/API y cómo se ejecutaron las pruebas.
Los revisores escanean las diferencias de la API exportada, el manejo de errores, la concurrencia y las diferencias de dependencias antes de aprobar.
Los PRs grandes detienen el trunk; divide por paquete o feature flag cuando sea posible.
Tarjeta de receta de referencia rápida, lista para copiar y pegar.
Título del PR: internal/parse: rechazar entrada vacía en Decode
Plantilla del cuerpo del PR:
## Resumen
- Rechazar entrada vacía en `Decode` con `ErrEmptyInput`
## Impacto en el módulo
- Sin nuevas dependencias
- La variable de error exportada es una adición compatible
## Plan de pruebas
- [ ] go test ./...
- [ ] go vet ./...
- [ ] go mod tidy (sin diferencias)
## Capturas de pantalla / logs
N/ACuándo usar esto:
Un contribuyente abre un PR añadiendo comprobaciones de salud gRPC:
// internal/health/grpc.go (extracto)
package health
import "google.golang.org/grpc/health/grpc_health_v1"
func Register(s *grpc.Server) {
srv := grpc_health_v1.NewHealthServer()
grpc_health_v1.RegisterHealthServer(s, srv)
}# Extracto de go.mod
+ google.golang.org/grpc v1.68.0Lista de verificación del revisor:
go test ./... está en verde en CI.internal/ fuera del límite del módulo.Aprobación después de que el autor documente el flag de exclusión y la comprobación de tidy se complete.
Lo que esto demuestra:
go.mod como de primera clase, no como ruido.main.La cola de fusión de GitHub (opcional) serializa las fusiones para mantener el trunk en verde bajo carga.
| Tamaño | Líneas (aprox.) | Resultado de la revisión |
|---|---|---|
| Pequeño | menos de 200 | Fusión el mismo día probable |
| Mediano | 200-400 | Necesita tiempo de revisión enfocado |
| Grande | 400+ | Dividir a menos que sea un renombre mecánico |
Los cambios mecánicos (go fix, actualización de protobuf generada) pueden exceder los límites si están aislados y etiquetados.
| Área | Pregunta |
|---|---|
| Símbolos exportados | ¿Esta promesa de compatibilidad para los consumidores del módulo? |
go.mod | ¿Nuevo peso transitivo? ¿Versión justificada? |
internal/ | ¿Alguna importación interna ilegal entre módulos? |
| Errores | ¿Envueltos con %w? ¿Errores centinela vs dinámicos documentados? |
| Concurrencia | ¿Ciclo de vida de goroutine, cancelación de contexto, carreras de datos? |
| Pruebas | ¿Basadas en tablas? ¿Cubren rutas de error? ¿Vale la pena -race? |
/v2 o un bump mayor?benchstat.Solicitar cambios por corrección, seguridad del módulo o pruebas faltantes.
Los comentarios de nit (detalles menores) deben ser opcionales (prefijo nit:).
Aprobar cuando los problemas son menores y confiar en el autor para seguimientos de deuda.
go.sum en bloque - Podría ocultar un intercambio en la cadena de suministro. Solución: lee go mod why -m para módulos nuevos.gh pr checkout o confía en la matriz de CI.| Alternativa | Usar Cuando | No Usar Cuando |
|---|---|---|
| Programación en parejas | Picos de diseño complejos | Equipo distribuido asíncrono |
| PRs apilados | Cambios secuenciales dependientes | Revisores carecen de herramientas de apilamiento |
| CODEOWNERS solicitud automática | Propiedad de monorepo grande | Biblioteca pequeña de dos personas |
| Fusión del mantenedor sin PR | Hotfix de emergencia | Flujo de características normal |
Apunta a un cambio lógico que los revisores puedan retener en la memoria de trabajo.
Separa refactors de cambios de comportamiento en PRs separados.
Para cambios arriesgados, sí: gh pr checkout <n> luego go test ./....
CI en verde es necesario pero no siempre suficiente.
Verifica el changelog y los avisos de seguridad para los módulos actualizados.
Ejecuta pruebas e inspecciona go mod why para rutas inesperadas.
breaking, module-deps, needs-changelog, security.
Filtra notas de lanzamiento y prioridad de revisión.
Prefiere un PR de renombre mecánico separado de los cambios de comportamiento.
Revisión más fácil y bisect más limpio.
Usa PRs draft.
Convierte a listo solo cuando la descripción, las pruebas y tidy estén completos.
Comenta en las entradas del generador o en los objetivos de Makefile, no directamente en las líneas de *.pb.go.
Regenera en un commit de seguimiento.
Rastrea las rutas de llamada desde los manejadores HTTP/gRPC hasta el trabajo en segundo plano.
Las comprobaciones faltantes de ctx.Done() son bloqueadores de lanzamiento.
Sí, para la API exportada o el comportamiento del que dependen los consumidores.
Las aplicaciones pueden usar solo notas de lanzamiento en el momento de la etiqueta.
Dentro de un día hábil mantiene el flujo del trunk saludable.
Establece un SLA de equipo y rota el deber de revisión.
Está bien cuando las comprobaciones requeridas son sólidas y se aplican los límites de tamaño de PR.
Evita para actualizaciones de dependencias sin una mirada humana.
Enumera cada go.mod tocado.
Confirma que las directivas de workspace o replace no se hayan comprometido accidentalmente.
Versiones de Stack: Esta página fue escrita para Go 1.26.x (predeterminado Green Tea GC, modernizadores de go fix - verificar parche en la compilación), chi (última - verificar en la compilación), gin (última - verificar en la compilación), echo (última - verificar en la compilación), google.golang.org/grpc (última - verificar en la compilación), sigs.k8s.io/controller-runtime (última - verificar en la compilación), kubebuilder (última - verificar en la compilación), tinygo (última - verificar objetivos de placa en la compilación), wazero (última - verificar en la compilación), y golangci-lint (última - verificar conjunto de linters en la compilación).
Revisado por Chris St. John·Última actualización: 19 jul 2026