Cultura de Revisión de Código para Go
Normas de revisión, directrices para 'nits' y enseñanza a través de PRs.
//
Busca en todas las páginas de la documentación
Normas de revisión, directrices para 'nits' y enseñanza a través de PRs.
Los pull requests son el aula principal para los modismos de Go en la mayoría de los equipos.
Una cultura de revisión saludable detecta errores temprano, difunde conocimiento y mantiene las APIs estables sin agotar a los autores o revisores.
La cultura de revisión de Go equilibra la fusión rápida con la alta confianza.
Los revisores se centran en el comportamiento ante fallos, la seguridad de la concurrencia, las promesas de la API exportada y los puntos de control operacionales.
Las preferencias cosméticas ceden ante gofmt y los linters.
La enseñanza ocurre en comentarios con justificación y enlaces, no en demandas sin explicación.
Tarjeta de referencia rápida - lista para copiar y pegar.
## Lista de verificación de descripción de PR (autor)
- [ ] Incidencia o ticket enlazado
- [ ] go test ./... y golangci-lint run (pegar o enlace de CI)
- [ ] -race run si se tocan goroutines, mapas compartidos entre goroutines o primitivas de sincronización
- [ ] Anotar cambios de API que rompen y pasos de migración
- [ ] Capturas de pantalla o líneas de ejemplo de registro para cambios observables en el comportamientoCuándo usar esto:
El revisor examina un PR de manejador que devuelve errores incorrectamente.
// Antes - el revisor bloquea: envuelve sin %w, registra y devuelve el mismo err
func (s *Server) fetchUser(ctx context.Context, id string) (*User, error) {
u, err := s.repo.Get(ctx, id)
if err != nil {
log.Printf("get user: %v", err)
return nil, fmt.Errorf("get user: %v", err)
}
return u, nil
}// Después - política de manejo única: envolver con %w, mapear a HTTP en el borde del manejador
func (s *Server) fetchUser(ctx context.Context, id string) (*User, error) {
u, err := s.repo.Get(ctx, id)
if err != nil {
return nil, fmt.Errorf("fetch user %s: %w", id, err)
}
return u, nil
}
func (s *Server) handleUser(w http.ResponseWriter, r *http.Request) {
u, err := s.fetchUser(r.Context(), r.PathValue("id"))
if err != nil {
if errors.Is(err, ErrNotFound) {
http.NotFound(w, r)
return
}
http.Error(w, "internal error", http.StatusInternalServerError)
return
}
json.NewEncoder(w).Encode(u)
}Lo que esto demuestra:
%w para errors.Is en los límites.ErrNotFound.| Prioridad | Buscar | Comentario de ejemplo |
|---|---|---|
| P0 | Carreras de datos, pánicos en rutas activas | "Mapa compartido sin mutex; ejecutar -race" |
| P1 | Semántica de errores incorrecta | "Usar %w para que el manejador pueda errors.Is" |
| P2 | Superficie de API/exportación | "Este tipo es público; ¿godoc y estabilidad?" |
| P3 | Pruebas faltantes en cambios de comportamiento | "Añadir subtest para la ruta de cancelación" |
| P4 | Observabilidad | "Los 5xx deberían registrar el ID de la solicitud" |
| P5 | Nomenclatura/estilo | "Considerar renombrar" solo si es confuso |
// Los revisores esperan el contexto como primer parámetro después del receptor
func (c *Client) Do(ctx context.Context, req *Request) (*Response, error)
// Marcar el almacenamiento de contexto en structs
type Bad struct {
ctx context.Context // revisión: no almacenar contexto
}Enseña aceptar interfaces, devolver structs mostrando una interfaz mínima definida por el consumidor en archivos de prueba.
staticcheck a través de golangci-lint.sync, canales o errgroup.| Alternativa | Usar Cuando | No Usar Cuando |
|---|---|---|
| Emparejamiento antes de enviar | El autor es nuevo en concurrencia | Cambio trivial solo de documentación |
| Revisión en grupo (Mob review) | Corrección de incidentes de aprendizaje | Pequeños PRs diarios |
| CODEOWNERS asignación automática | Difundir conocimiento del dominio | Único guardián en todos los archivos |
| Linter como único guardián | 'Nits' repetidos objetivos | Juicios sobre diseño de API |
Bloquear por corrección y ADRs del equipo. Enviar 'nits' como sugerencias opcionales a menos que estén automatizados. Promover 'nits' repetidos a linters.
Para cambios arriesgados, sí. Para diferencias pequeñas con CI en verde, leer las diferencias de pruebas puede ser suficiente. Confiar pero verificar en concurrencia.
El mismo estándar que el código humano. El autor debe explicar el diseño; los revisores vigilan la concurrencia plausible pero incorrecta y los malos saltos de módulo.
Usar para seguimientos no bloqueantes que se rastrean en tickets. Nunca para elementos P0-P1.
Fomentar preguntas y comentarios sobre la legibilidad de las pruebas. Los seniors modelan respuestas amables para construir seguridad psicológica.
Para nuevas exportaciones, sí. Para refactorizaciones internas, corregir godoc en el mismo PR o archivar un seguimiento antes de la próxima etiqueta de lanzamiento.
Sincronización corta o pico de ADR. Por defecto, usar el estándar del equipo documentado hasta que el ADR cambie.
Primera respuesta dentro de un día hábil; revisión completa dentro de dos para PRs medianos. Escalar bloqueos explícitamente.
Sí, para elecciones específicas del equipo: mapeo de errores, diseño de módulos, nombres de campos de observabilidad, patrones de frameworks.
Se espera que los seniors dejen comentarios educativos y mejoren las directrices según la Guía de Nivelación.
Versiones de Stack: Esta página fue escrita para Go 1.26.x (predeterminado Green Tea GC, go fix modernizers - verificar parche en la compilación), chi (última versión - verificar en la compilación), gin (última versión - verificar en la compilación), echo (última versión - verificar en la compilación), google.golang.org/grpc (última versión - verificar en la compilación), sigs.k8s.io/controller-runtime (última versión - verificar en la compilación), kubebuilder (última versión - verificar en la compilación), tinygo (última versión - verificar objetivos de placa en la compilación), wazero (última versión - verificar en la compilación), y golangci-lint (última versión - verificar conjunto de linters en la compilación).
Revisado por Chris St. John·Última actualización: 16 jul 2026