Azure Repos: revisar una pull request

Revisar no es leer el diff de arriba abajo. Es buscar lo que rompe en producción y decirlo donde el autor pueda actuar.

1. Traer el contexto

az repos pr show --id <PR> --output json

De ahí salen sourceRefName, targetRefName, repository.id y los workItemRefs (el PBI que justifica el cambio: si no hay ninguno, ya tienes el primer comentario).

El diff se lee mejor en local. Azure Repos publica la rama de la PR en refs/pull/<id>/merge:

git fetch origin refs/pull/<PR>/merge
git diff origin/<rama-destino>...FETCH_HEAD
git diff --stat origin/<rama-destino>...FETCH_HEAD   # empieza por aquí

Lee el --stat primero. Decide dónde mirar antes de mirar: 40 ficheros cambiados con 38 de traducciones y 2 de lógica son una PR de 2 ficheros.

2. Qué se busca, en este orden

Correctitud — lo único que justifica bloquear una PR:

Condiciones de contorno: colección vacía, null, valor límite, primera ejecución, reintento.
async sin await, .Result o .Wait() (deadlock esperando su turno), CancellationToken que se pierde por el camino.
Concurrencia: estado compartido mutable, dos peticiones a la vez sobre la misma fila sin control de concurrencia optimista.
Transacciones: ¿qué queda a medias si el paso 3 de 5 lanza excepción?
Consultas EF Core nuevas: N+1, AsNoTracking() ausente, filtro que se evalúa en cliente.

Contrato y compatibilidad — lo que rompe a otros:

Cambio en una respuesta pública de API: ¿campo eliminado o renombrado?
Migración de base de datos: ¿es reversible? ¿bloquea la tabla? ¿el despliegue admite versión antigua y nueva a la vez?
Configuración nueva sin valor por defecto → el entorno que no la tenga arranca y falla en la primera petición.

Seguridad:

Secretos, cadenas de conexión o tokens en el diff. Si aparece uno, el comentario no es “quítalo”: es “rótalo, ya está en el historial”.
Entrada del usuario que llega a SQL, a una ruta de fichero o a la respuesta HTML sin escapar.
Endpoint nuevo sin RequireAuthorization() o sin comprobar que el recurso pertenece a quien lo pide.

Pruebas:

El caso que motiva la PR, ¿está cubierto por un test que falle sin el arreglo? Si el bug puede volver sin que nadie se entere, la PR no está terminada.

Y después, lo demás: nombres, duplicación, complejidad. Sugerencias, no bloqueos.

3. Comentar donde toca

La CLI no crea hilos de comentarios; es REST. Un hilo anclado a una línea:

az devops invoke \
  --area git --resource pullRequestThreads \
  --route-parameters project=$PROJECT repositoryId=$REPO pullRequestId=$PR \
  --api-version 7.1 --http-method POST --in-file hilo.json

{
  "comments": [{
    "parentCommentId": 0,
    "content": "`items` puede venir vacío y `First()` lanza. ¿`FirstOrDefault()` y salida temprana?",
    "commentType": "text"
  }],
  "status": "active",
  "threadContext": {
    "filePath": "/src/Application/Orders/OrderService.cs",
    "rightFileStart": { "line": 42, "offset": 1 },
    "rightFileEnd":   { "line": 42, "offset": 30 }
  }
}

filePath empieza por / y es relativo a la raíz del repo.
rightFile* para líneas añadidas, leftFile* para las eliminadas.
status: "active" abre el hilo (visible como pendiente); "closed" lo deja como comentario informativo que no estorba.

4. El comentario de cierre

Un resumen en el hilo general con el veredicto, sin ambigüedad:

Revisado. 2 bloqueantes (#42 vacío, migración no reversible),
3 sugerencias. El resto, bien: la separación de OrderService quedó limpia.

Reglas de tono

Comenta el código, nunca a la persona: “esto se rompe si X”, no “no has tenido en cuenta X”.
Distingue explícitamente bloqueante de sugerencia. Un revisor que no prioriza obliga al autor a adivinar.
Si el diff es correcto, dilo y aprueba. Inventar pegas para justificar la revisión es la forma más rápida de que dejen de pedírtelas.
Nunca apruebes ni votes en nombre de otra persona: el veredicto se propone, lo firma quien revisa.

azure-pr-review

Cómo se instala

El fichero, entero