(#9) Add PR template #10
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| <!-- IMPORTANTE: Lee estos comentarios sobre cómo crear PR que enamoren --> | ||
|
|
||
| <!-- Nombra (o renombra) la rama con el siguiente esquema: <GIID>-<feature-name> o <GIID>-<bug-fix> --> | ||
| <!-- GIID: Github Issue ID--> | ||
| <!-- Ejemplo: 01-add-github-template --> | ||
|
|
||
| <!-- Deberías escribir el nombre de la funcionalidad o bug fix en el título de la PR, prefijando el (#GIID) con el ID de la issue relacionada --> | ||
|
|
||
| <!-- Cuando hagas un commit, haz commits pequeños, pensando en la funcionalidad conseguida, utilizando verbos --> | ||
| <!-- Ejemplo: "Configure Travis CI integration" --> | ||
|
|
||
| ## ¿Qué hace esta PR? | ||
| <!-- Describe brevemente qué se ha implementado en esta PR --> | ||
|
|
||
| ## ¿Por qué es importante? | ||
| <!-- Proporciona contexto sobre por qué son necesarios estos cambios --> | ||
|
|
||
| ## ¿Cómo se prueba esta PR? | ||
| <!-- Proporciona contexto sobre cómo probar esta PR: casos de uso, rangos de valores, etc --> | ||
|
|
||
| ## Tipo de Cambio | ||
| <!-- Por favor elimina las opciones que no sean relavantes en esta PR --> | ||
|
|
||
| - [ ] Nueva funcionalidad | ||
| - [ ] Bug fix | ||
| - [ ] Mejora | ||
| - [ ] Breaking change | ||
| - [ ] Require actualización de la documentación | ||
|
|
||
| ## Checklist: | ||
| <!-- Marca aquellos elementos satisfechos por esta PR, y deja sin marcar aquéllos no satisfechos --> | ||
|
|
||
| - [ ] Mi código sigue las normas de estilo del proyecto | ||
| - [ ] He realizado una auto-revisión de mi código | ||
| - [ ] Mi código expresa por sí sólo lo que quiere hacer, y sólo están comentados que aquellos fragmentos que lo necesitan | ||
| - [ ] He actualizado la documentación | ||
| - [ ] Mis cambios no generan nuevos warnings | ||
| - [ ] He añadido tests que prueban, o bien que mi fix es efectivo, o que la funcionalidad funciona como se espera | ||
|
There was a problem hiding this comment. Hay algún caso en el que los cambios puedan generar nuevos warnings y pueda ser mergeada? There was a problem hiding this comment. Creo que el éxito está en conseguir que la persona que envía una PR se responsabilice de no enviar una PR como Billy el niño, así de botonazo caliente. Al tener que rellenar este template debería hacerle pensar sobre lo que está enviando There was a problem hiding this comment. Si usaramos el caso de arriba semantic commits, no haría falta este check (se podría ver con la etiqueta test en los commits). Aunque no me opongo si lo véis mas claro así. There was a problem hiding this comment. Es lo mismo, responsabilizar a quien envía la PR para que pruebe en local There was a problem hiding this comment. con el pre-commit esto estaría cubierto, otra cosa sería algo así como, También añadiría algo como There was a problem hiding this comment. Este check es para verificar que se ha probado en local: he arrancado la aplicación, sus dependencias, y pruebo el código, por tanto los tests son fiables. No dejarse llevar por el efecto "tiene buena pinta". There was a problem hiding this comment. Creo que es importante añadir lo que ha dicho @javierlopezdeancos para auto revisar que la PR está correctamente documentada.
|
||
| - [ ] Los tests (nuevos y existentes) pasan localmente tras mis cambios | ||
mdelapenya marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| - [ ] Los cambios dependientes han sido mergeados y publicados en los proyectos relacionados (downstrean projects) | ||
| - [ ] Creo que la descripción de esta PR tiene la documentación necesaria, capturas y/o videos, para entender su contexto | ||
|
|
||
| ## Issues relacionadas | ||
|
|
||
| <!-- Enlaza las issues relacionadas aquí abajo. Inserta un enlace a la issue o escribe las palabras "Closes X" si mergeando esta PR debería cerrar automáticamente una issue. --> | ||
|
|
||
| ## Checklist del/a Autor/a | ||
|
|
||
| <!-- Añade una lista de cosas que sea necesarias para ser revisadas para que esta PR sea aprobada --> | ||
|
|
||
| - [ ] | ||
|
|
||
| <!-- No olvides poner a alguien del GDG Toledo como revisor --> | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
En go no se formatea el codigo automaticamente? en el caso de no u otros lenguajes, soy de actualizar la tarea, dejar esta responsabilidad al desarrollador no me gusta sobre todo cuando es una tarea de 10 min que se tarda en automatizar, al menos en js con prettier.
La democracia a fracasado, mirad Grecia o las 4 elecciones en 4 años de nuestro país jajaja 🎩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Normas de estilo no tiene por qué ser el tabulado, la posición del paréntesis, los espacios. Para mí, además de eso, es el nombrado de variables, el uso de ciertos patrones, etc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Y sí, Go se formatea sólo, pero voy más por el segundo nivel, el del estilo de programación
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
De acuerdo contigo, tenemos las
normas de estilo del proyecto, quiero decir, este check será check o uncheck con respecto a unos criterios no?. Lo digo por pensar en devs externos o que pasan de manera tangencial en el proyecto, de como obtener todo el contexto.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exacto, en algún momento tendríamos un ficherito con las
normas de estilo. Pero vamos, que estamos de momento bastante lejos de ello (en este proyecto)