{
 "cells": [
  {
   "cell_type": "markdown",
   "metadata": {},
   "source": [
    "# Revisar el trabajo de otra persona\n",
    "\n",
    "Cómo se lee un cambio, qué se comenta y por qué hay que bajarlo\n",
    "\n",
    "Cuaderno de práctica del capítulo 23 de **Git desde cero**, de Miss Yera.\n",
    "\n",
    "Corre de arriba abajo. Si lo abres en Google Colab no necesitas instalar nada.\n",
    "\n",
    "Capítulo completo: https://missyera.com/guias/git-desde-cero/revisar-el-trabajo-ajeno/\n",
    "\n",
    "Los ejercicios están al final y traen una celda vacía debajo de cada uno. Las\n",
    "respuestas viven en el cuaderno de soluciones, y merece la pena pelearse un\n",
    "rato antes de abrirlo 💛"
   ]
  },
  {
   "cell_type": "markdown",
   "metadata": {},
   "source": [
    "## Antes de empezar\n",
    "\n",
    "Este capítulo son comandos de terminal, no Python. La celda de abajo baja el\n",
    "ayudante que los ejecuta y que **recuerda en qué carpeta quedaste**, que es lo\n",
    "que hace falta para que un `cd` de una celda siga valiendo en la siguiente.\n",
    "\n",
    "A partir de ahí, cada celda de comandos empieza por `%%consola`."
   ]
  },
  {
   "cell_type": "code",
   "execution_count": null,
   "metadata": {},
   "outputs": [],
   "source": [
    "import urllib.request\n",
    "\n",
    "# El ayudante de los cuadernos. Trae la corrección de los ejercicios y, en los\n",
    "# capítulos de consola, la celda mágica que ejecuta los comandos. Se baja en\n",
    "# vez de venir pegado aquí para que siempre sea el último.\n",
    "urllib.request.urlretrieve(\n",
    "    \"https://missyera.com/static/cuadernos/revisa.py\", \"revisa.py\")\n",
    "import revisa\n",
    "revisa.carga({\n",
    "    1: \"M2I2MTgxNyBTZSBhZ3JlZ2EgZWwgY2F0YWxvZ28gZGUgY2xpZW50ZXMKNTRkYTZlMiBQcmltZXJhcyB2ZW50YXMgZGUgTGltYQ==\",\n",
    "    2: \"IGNsaWVudGVzLmNzdiB8IDIgKysKIDEgZmlsZSBjaGFuZ2VkLCAyIGluc2VydGlvbnMoKyk=\",\n",
    "    3: \"ZGlmZiAtLWdpdCBhL2NsaWVudGVzLmNzdiBiL2NsaWVudGVzLmNzdgpuZXcgZmlsZSBtb2RlIDEwMDY0NAppbmRleCAwMDAwMDAwLi41YjY0ODdmCi0tLSAvZGV2L251bGwKKysrIGIvY2xpZW50ZXMuY3N2CkBAIC0wLDAgKzEsMiBAQAorY2xpZW50ZSxjaXVkYWQKK0JvZGVnYSBJbnRpLEN1c2Nv\",\n",
    "    4: \"bGltcGlv\",\n",
    "    5: \"Y2xpZW50ZXMuY3N2CnZlbnRhcy5jc3YKY2xpZW50ZSxjaXVkYWQKQm9kZWdhIEludGksQ3VzY28KdmVudGFzLmNzdg==\",\n",
    "    6: \"MjdlMGM5NCBTZSBhY2VwdGEgZWwgY2F0YWxvZ28gZGUgY2xpZW50ZXMKNTRkYTZlMiBQcmltZXJhcyB2ZW50YXMgZGUgTGltYQozYjYxODE3IFNlIGFncmVnYSBlbCBjYXRhbG9nbyBkZSBjbGllbnRlcwpjbGllbnRlcy5jc3YKdmVudGFzLmNzdg==\",\n",
    "    7: \"ZmF0YWw6IGFtYmlndW91cyBhcmd1bWVudCAnbWFpbi4uYWdyZWdhLWNsaWVudGUnOiB1bmtub3duIHJldmlzaW9uIG9yIHBhdGggbm90IGluIHRoZSB3b3JraW5nIHRyZWUuClVzZSAnLS0nIHRvIHNlcGFyYXRlIHBhdGhzIGZyb20gcmV2aXNpb25zLCBsaWtlIHRoaXM6CidnaXQgPGNvbW1hbmQ+IFs8cmV2aXNpb24+Li4uXSAtLSBbPGZpbGU+Li4uXSc=\",\n",
    "}, lenguaje=\"bash\")"
   ]
  },
  {
   "cell_type": "markdown",
   "metadata": {},
   "source": [
    "Revisar el trabajo de otra persona es la habilidad que más rápido te hace\n",
    "útil en un equipo, y casi nadie la enseña. Se aprende mirando cómo lo hacen\n",
    "otros, que es una forma elegante de decir que se aprende mal 🙃"
   ]
  },
  {
   "cell_type": "markdown",
   "metadata": {},
   "source": [
    "## Lo que una revisión sí puede ver"
   ]
  },
  {
   "cell_type": "markdown",
   "metadata": {},
   "source": [
    "Y lo que no. Esta distinción es todo el capítulo:"
   ]
  },
  {
   "cell_type": "markdown",
   "metadata": {},
   "source": [
    "| Leyendo el diff | Solo bajándolo y ejecutando |\n",
    "|---|---|\n",
    "| Si el cambio hace lo que dice el título | Si funciona con los datos reales |\n",
    "| Si entró algo que no debía (una clave, un archivo suelto) | Si rompió otra cosa que ya andaba |\n",
    "| Si los nombres se entienden | Si la salida es la que se esperaba |\n",
    "| Si falta documentar algo | Cuánto tarda |"
   ]
  },
  {
   "cell_type": "markdown",
   "metadata": {},
   "source": [
    "## Primero, leer"
   ]
  },
  {
   "cell_type": "markdown",
   "metadata": {},
   "source": [
    "Montamos un proyecto donde alguien propone un cambio en una rama:"
   ]
  },
  {
   "cell_type": "code",
   "execution_count": null,
   "metadata": {},
   "outputs": [],
   "source": [
    "%%consola\n",
    "mkdir proyecto\n",
    "cd proyecto\n",
    "git init -q\n",
    "printf 'ciudad,monto\\nLima,1200\\nArequipa,890\\n' > ventas.csv\n",
    "printf '# Reporte de ventas\\n' > README.md\n",
    "git add .\n",
    "git commit -q -m \"Primera version del reporte de ventas\"\n",
    "git switch -q -c agrega-canales\n",
    "printf 'canal,monto\\nBodegas,4200\\nHoreca,3100\\n' > canales.csv\n",
    "printf 'Cusco,760\\n' >> ventas.csv\n",
    "git add .\n",
    "git commit -q -m \"Se agrega el reporte por canal y la ciudad de Cusco\"\n",
    "git switch -q main\n",
    "git log --oneline --all"
   ]
  },
  {
   "cell_type": "markdown",
   "metadata": {},
   "source": [
    "Lo primero que miro nunca es el código: es **el tamaño**."
   ]
  },
  {
   "cell_type": "code",
   "execution_count": null,
   "metadata": {},
   "outputs": [],
   "source": [
    "%%consola\n",
    "git diff --stat main..agrega-canales"
   ]
  },
  {
   "cell_type": "markdown",
   "metadata": {},
   "source": [
    "Dos archivos y pocas líneas se revisa bien. Cuarenta archivos y mil líneas\n",
    "no se revisa, se aprueba por cansancio, y esa es la principal causa de\n",
    "revisiones inútiles. Cuando veas algo así, lo que corresponde es pedir que se\n",
    "parta en varias propuestas 🔪"
   ]
  },
  {
   "cell_type": "markdown",
   "metadata": {},
   "source": [
    "## Después, el contenido"
   ]
  },
  {
   "cell_type": "code",
   "execution_count": null,
   "metadata": {},
   "outputs": [],
   "source": [
    "%%consola\n",
    "git diff main..agrega-canales"
   ]
  },
  {
   "cell_type": "markdown",
   "metadata": {},
   "source": [
    "Las líneas con `+` entran y las de `-` salen, igual\n",
    "que en 6. Aquí ya se puede opinar con fundamento."
   ]
  },
  {
   "cell_type": "markdown",
   "metadata": {},
   "source": [
    "## Y lo que casi nadie hace: bajarla"
   ]
  },
  {
   "cell_type": "markdown",
   "metadata": {},
   "source": [
    "En un pull request de GitHub la rama ya está en tu remoto, así que son dos\n",
    "comandos:"
   ]
  },
  {
   "cell_type": "code",
   "execution_count": null,
   "metadata": {},
   "outputs": [],
   "source": [
    "%%consola\n",
    "git switch -q agrega-canales\n",
    "ls\n",
    "cat canales.csv\n",
    "git log --oneline -1"
   ]
  },
  {
   "cell_type": "markdown",
   "metadata": {},
   "source": [
    "Ahora tienes los archivos de la propuesta en tu carpeta y puedes ejecutar lo\n",
    "que haga falta. Cuando terminas, vuelves a lo tuyo con\n",
    "`git switch main` y no queda rastro 🔙"
   ]
  },
  {
   "cell_type": "markdown",
   "metadata": {},
   "source": [
    "## Qué comentar y cómo"
   ]
  },
  {
   "cell_type": "markdown",
   "metadata": {},
   "source": [
    "La regla que más me ha servido: **separa lo que bloquea de lo que es\n",
    "gusto**. Si todo suena igual de grave, quien recibe la revisión no sabe\n",
    "por dónde empezar y se desanima.\n",
    "\n",
    "- **Bloquea:** \"esto rompe el reporte cuando el CSV viene\n",
    "vacío\".\n",
    "\n",
    "- **Sugerencia:** \"yo llamaría `monto_total` a esa\n",
    "columna, pero como está funciona\".\n",
    "\n",
    "- **Pregunta:** \"¿por qué se filtra Lima aquí y no antes?\".\n",
    "\n",
    "Y se comenta el código, no a la persona. \"Esta función no maneja el caso de\n",
    "la bodega sin ventas\" y no \"no pensaste en el caso de la bodega sin\n",
    "ventas\" 🙂"
   ]
  },
  {
   "cell_type": "markdown",
   "metadata": {},
   "source": [
    "## Aceptar la propuesta"
   ]
  },
  {
   "cell_type": "code",
   "execution_count": null,
   "metadata": {},
   "outputs": [],
   "source": [
    "%%consola\n",
    "git switch -q main\n",
    "git merge -q --no-ff -m \"Se acepta el reporte por canal\" agrega-canales\n",
    "git log --oneline\n",
    "ls"
   ]
  },
  {
   "cell_type": "markdown",
   "metadata": {},
   "source": [
    "Ese `--no-ff` obliga a dejar un commit de fusión aunque no hiciera\n",
    "falta, y eso deja escrito en la historia que hubo una propuesta y que se\n",
    "aceptó. En equipos es lo que quieres 📌"
   ]
  },
  {
   "cell_type": "markdown",
   "metadata": {},
   "source": [
    "### La trampa\n",
    "\n",
    "Te toca revisar el pull request de un compañero. Lees el diff en GitHub, se ve bien, apruebas.\n",
    "\n",
    "```\n",
    "$ git log --oneline -1\n",
    "9f3c2a1 Se acepta el reporte por canal\n",
    "\n",
    "# tres dias despues\n",
    "$ python3 reporte.py canales.csv\n",
    "KeyError: 'monto'\n",
    "```\n",
    "\n",
    "**¿Qué está mal?** La respuesta está en el cuaderno de soluciones. Míralo tú primero."
   ]
  },
  {
   "cell_type": "markdown",
   "metadata": {},
   "source": [
    "## Comprueba que se entendió"
   ]
  },
  {
   "cell_type": "markdown",
   "metadata": {},
   "source": [
    "### Comprueba que lo tienes\n",
    "\n",
    "Te toca revisar un pull request de 40 archivos y 1200 líneas. ¿Qué haces?\n",
    "\n",
    "a) Lo leo entero con calma, aunque me tome la tarde\n",
    "\n",
    "b) Pido que se parta en propuestas más chicas\n",
    "\n",
    "c) Lo apruebo, si tiene tantos archivos seguro está pensado\n",
    "\n",
    "d) Reviso solo los archivos que conozco"
   ]
  },
  {
   "cell_type": "markdown",
   "metadata": {},
   "source": [
    "## Ejercicios"
   ]
  },
  {
   "cell_type": "markdown",
   "metadata": {},
   "source": [
    "### 1. Monta una propuesta para revisar\n",
    "\n",
    "Un proyecto con una rama que agrega el catálogo de\n",
    "clientes."
   ]
  },
  {
   "cell_type": "code",
   "execution_count": null,
   "metadata": {},
   "outputs": [],
   "source": [
    "%%revisa 1\n",
    "# tu turno"
   ]
  },
  {
   "cell_type": "markdown",
   "metadata": {},
   "source": [
    "### 2. Mira el tamaño antes que nada\n",
    "\n",
    "Cuántos archivos y cuántas líneas cambian."
   ]
  },
  {
   "cell_type": "code",
   "execution_count": null,
   "metadata": {},
   "outputs": [],
   "source": [
    "%%revisa 2\n",
    "# tu turno"
   ]
  },
  {
   "cell_type": "markdown",
   "metadata": {},
   "source": [
    "### 3. Lee el cambio\n",
    "\n",
    "El contenido exacto de la propuesta."
   ]
  },
  {
   "cell_type": "code",
   "execution_count": null,
   "metadata": {},
   "outputs": [],
   "source": [
    "%%revisa 3\n",
    "# tu turno"
   ]
  },
  {
   "cell_type": "markdown",
   "metadata": {},
   "source": [
    "### 4. Comprueba que no entró nada de más\n",
    "\n",
    "La revisión que hago siempre: buscar claves en lo que\n",
    "entra."
   ]
  },
  {
   "cell_type": "code",
   "execution_count": null,
   "metadata": {},
   "outputs": [],
   "source": [
    "%%revisa 4\n",
    "# tu turno"
   ]
  },
  {
   "cell_type": "markdown",
   "metadata": {},
   "source": [
    "### 5. Bájala y ejecútala\n",
    "\n",
    "Cámbiate a la rama para tener los archivos delante."
   ]
  },
  {
   "cell_type": "code",
   "execution_count": null,
   "metadata": {},
   "outputs": [],
   "source": [
    "%%revisa 5\n",
    "# tu turno"
   ]
  },
  {
   "cell_type": "markdown",
   "metadata": {},
   "source": [
    "### 6. Acepta la propuesta dejando rastro\n",
    "\n",
    "Fusiona con `--no-ff` para que quede escrito\n",
    "que hubo una propuesta."
   ]
  },
  {
   "cell_type": "code",
   "execution_count": null,
   "metadata": {},
   "outputs": [],
   "source": [
    "%%revisa 6\n",
    "# tu turno"
   ]
  },
  {
   "cell_type": "markdown",
   "metadata": {},
   "source": [
    "### 7. Intenta revisar una rama que no existe\n",
    "\n",
    "Pide el diff contra un nombre mal escrito, que es lo que\n",
    "pasa cuando copias el nombre de la rama con un dedazo."
   ]
  },
  {
   "cell_type": "code",
   "execution_count": null,
   "metadata": {},
   "outputs": [],
   "source": [
    "%%revisa 7\n",
    "# tu turno"
   ]
  },
  {
   "cell_type": "markdown",
   "metadata": {},
   "source": [
    "## Lo que te llevas"
   ]
  },
  {
   "cell_type": "markdown",
   "metadata": {},
   "source": [
    "**Leer dice si es razonable; ejecutar dice si funciona. Y separa lo\n",
    "que bloquea de lo que es gusto.**"
   ]
  },
  {
   "cell_type": "markdown",
   "metadata": {},
   "source": [
    "---\n",
    "\n",
    "Ese era el capítulo 23 de **Git desde cero**. El texto completo, con las salidas de cada bloque, está en https://missyera.com/guias/git-desde-cero/revisar-el-trabajo-ajeno/\n",
    "\n",
    "Que tengas lindo día! 🌸"
   ]
  }
 ],
 "metadata": {
  "kernelspec": {
   "display_name": "Python 3",
   "language": "python",
   "name": "python3"
  },
  "language_info": {
   "name": "python",
   "version": "3.11"
  }
 },
 "nbformat": 4,
 "nbformat_minor": 5
}
