Template element#384
Conversation
joaquinelio
left a comment
There was a problem hiding this comment.
Muy, bueno, encontré solo un acento y un signo ! de apertura
Me permití arreglar los números de linea. Necesitamos que coincidan para facilitar la review y epecialmente los sync con el repo inglés.
¡AY! casi se me escapan: Acá no afectan porque en el ejemplo no lo usan, pero:
no se traducen variables, ni classes y id del html (se llama refactoring ja!)
podrian ntroducir fallos, especialmente si se usaran en css y js externos
|
Please make the requested changes. After it, add a comment "/done". |
Co-authored-by: joaquinelio <[email protected]>
Co-authored-by: joaquinelio <[email protected]>
Co-authored-by: joaquinelio <[email protected]>
Co-authored-by: joaquinelio <[email protected]>
Co-authored-by: joaquinelio <[email protected]>
Co-authored-by: joaquinelio <[email protected]>
|
/done |
Co-authored-by: joaquinelio <[email protected]>
|
👍 👍 |
vplentinax
left a comment
There was a problem hiding this comment.
Muy bueno, corregí algunas estructuras de frases y eliminé comillas que no eran necesarias. Por lo demás, correcto. Esperaré los cambios para aprobarlo
|
Please make the requested changes. After it, add a comment "/done". |
|
Hola @RocioC1207 ! Te he dejado unas pequeñas correcciones para que las revises y podamos aprobar tu PR, en cuanto te sea posible, sólo queremos saber si aún sigues activa en la repo. Gracias! |
|
@joaquinelio Revisando este PR me di cuenta que viene de la rama "master"... ¿Afectará en algo eso? Creo que @RocioC1207 deberá rehacer el PR desde una nueva rama como hemos sugerido en el README |
|
Que sea SU master No nos afecta. |
|
FIRME recomendación es mantener el master propio limpio, pero solo es comodidad para uno. Entonces: Tenes TU fork Al Modificar el master Pero al PR no le importa de donde vienen los commits. Edito: Pero un nuevo PR sería sobre una base algo distinta, el cambio no se envía de nuevo Como sea, modificar master local es solo un incordio individual. |
|
Y si no aparece votaria por merge igual, |
No afectará en este PR. Para el próximo PR que cree sí afectará, porque creará nueva rama desde master y esta contendrá los commits que master tenga. Por ello la recomendación en README. Lo mejor es que una vez aprobado este PR, @RocioC1207 elimine su fork y su repo local, y vuelva a crearlo desde 0. Así su master estará limpio y comenzará siguiendo nuestra recomendación: Nueva rama para cada nuevo PR |
|
Ah, perdón @vplentinax, contestaba a "si deberá rehacer el PR" y no, no haria falta. Tambien puse "se arregla fácil" no puse cómo git reset si trabaja local, si edita online ni idea, pero en ese caso borrar y rehacer el fork despues de este PR sería muy simple. |
|
Y parece que @RocioC1207 se cansó de nosotros... =) |
Co-authored-by: Valentina VP <[email protected]>
Co-authored-by: Valentina VP <[email protected]>
Co-authored-by: Valentina VP <[email protected]>
Co-authored-by: Valentina VP <[email protected]>
|
Hola! |
|
/done |
|
@danilobrinu gracias por tomarte el tiempo de leerlo En este caso habia los dos reviews, una completa y la otra en proceso, por eso no di merge a pesar de tener tu aprobacion Tambien notaras que el bot no te registra. Los maintainers sí, a veces los pr quedan demorados por falta de review asi que siempre es bueno tener uno más que revise. |
|
Thank you 💖 I updated the Progress Issue #17 🎉 🎉 🎉 |
|
@joaquinelio sii recien me di cuenta, por cierto ya estoy libre para apoyar en los reviews :D |
|
@danilobrinu ok! |
Hola! Espero sus correcciones.
Saludos!