Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Popups and window methods #359

Merged
merged 36 commits into from
Sep 8, 2020

Conversation

Derlys
Copy link
Contributor

@Derlys Derlys commented Aug 25, 2020

No description provided.

@CLAassistant
Copy link

CLAassistant commented Aug 25, 2020

CLA assistant check
All committers have signed the CLA.

@joaquinelio joaquinelio changed the title Update 3-frames-and-windows/01-popup-windows/article.md Popups and window methods Aug 25, 2020
@joaquinelio joaquinelio self-requested a review August 25, 2020 22:24
Copy link
Member

@joaquinelio joaquinelio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uf que arduo fue esto
espero qie no te desanime =)

Conviene que el nombre del PR sea el del articulo, asi Don Bot lo registra. Igual anda perezoso y lo terminamos agregando a mano asi que no big deal.

Cuida los numeros de linea, facilita la review y ESPECIALMENTE los merge de actualizaciones del inglés.
Me atreví a corregirlos para esta review.
No agregues/quites lineas.

Tampoco deberias cambiar el estilo, veo que esspecialmente cambiaste comillas y pusiste esspacios, (y en ningun proyecto ajeno) aunque te parezca horrible ( salvo si es bug, que mandamos pr al inglés)
Por ej nunca uso ";" en mi codigo pero si es compartido me adapto a poner esa porquería inutil.

No modifiques codigo si no podes probarlo plis
Si te interesa podes instalar el server local para probar,
ademas asi se entiende como funcionan los markups

3-frames-and-windows/01-popup-windows/article.md Outdated Show resolved Hide resolved
3-frames-and-windows/01-popup-windows/article.md Outdated Show resolved Hide resolved
3-frames-and-windows/01-popup-windows/article.md Outdated Show resolved Hide resolved
3-frames-and-windows/01-popup-windows/article.md Outdated Show resolved Hide resolved
3-frames-and-windows/01-popup-windows/article.md Outdated Show resolved Hide resolved
3-frames-and-windows/01-popup-windows/article.md Outdated Show resolved Hide resolved
3-frames-and-windows/01-popup-windows/article.md Outdated Show resolved Hide resolved
3-frames-and-windows/01-popup-windows/article.md Outdated Show resolved Hide resolved
3-frames-and-windows/01-popup-windows/article.md Outdated Show resolved Hide resolved
3-frames-and-windows/01-popup-windows/article.md Outdated Show resolved Hide resolved
@javascript-translate-bot

Please make the requested changes. After it, add a comment "/done".
Then I'll ask for a new review 👻

EzequielCaste and others added 12 commits August 28, 2020 12:50
Co-authored-by: joaquinelio <joaquinelio@gmail.com>
Co-authored-by: joaquinelio <joaquinelio@gmail.com>
Co-authored-by: joaquinelio <joaquinelio@gmail.com>
Co-authored-by: joaquinelio <joaquinelio@gmail.com>
Co-authored-by: joaquinelio <joaquinelio@gmail.com>
Co-authored-by: joaquinelio <joaquinelio@gmail.com>
Co-authored-by: joaquinelio <joaquinelio@gmail.com>
Co-authored-by: joaquinelio <joaquinelio@gmail.com>
Co-authored-by: joaquinelio <joaquinelio@gmail.com>
Co-authored-by: joaquinelio <joaquinelio@gmail.com>
Co-authored-by: joaquinelio <joaquinelio@gmail.com>
Co-authored-by: joaquinelio <joaquinelio@gmail.com>
Copy link
Member

@joaquinelio joaquinelio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AAAHHHH!!!
Perdonperdonperdon ¡Me pasé por alto unos cambios importantes!
Muchos atributos que son parametros, no se traducen `width:200,height=100
Va de nuevo...

No corrijo formatos porque el original tampoco es homogeneo
además veo que el también empezó a agregar los ";"

3-frames-and-windows/01-popup-windows/article.md Outdated Show resolved Hide resolved
3-frames-and-windows/01-popup-windows/article.md Outdated Show resolved Hide resolved
3-frames-and-windows/01-popup-windows/article.md Outdated Show resolved Hide resolved
3-frames-and-windows/01-popup-windows/article.md Outdated Show resolved Hide resolved
3-frames-and-windows/01-popup-windows/article.md Outdated Show resolved Hide resolved
3-frames-and-windows/01-popup-windows/article.md Outdated Show resolved Hide resolved
3-frames-and-windows/01-popup-windows/article.md Outdated Show resolved Hide resolved
3-frames-and-windows/01-popup-windows/article.md Outdated Show resolved Hide resolved
3-frames-and-windows/01-popup-windows/article.md Outdated Show resolved Hide resolved
3-frames-and-windows/01-popup-windows/article.md Outdated Show resolved Hide resolved
@javascript-translate-bot

Please make the requested changes. After it, add a comment "/done".
Then I'll ask for a new review 👻

@joaquinelio
Copy link
Member

Reto:
Lo es. Fue tambien mi primera vez. y aprendi un monton de git y js en pocos meses así que te entiendo.

PR:
Nada importante.
Se debería poner el nombre exacto del articulo como nombre de PR, pero igual el bot a veces lo ignora y lo terminamos agregando a mano.

Comillas:
SEGURAMENTE a tu editor se le puede configurar el corrector de estilo o deshabilitarlo.
Es importante porque podriass trabajar en diferentes equipos con diferentes estilos y deberias respetarlos aunque sean una porquería.
Y acá especificamente porque mucho del codigo puede tener markup, parece que tu editor lo borró.

De nuevo al ruedo... 👍

Derlys and others added 10 commits September 2, 2020 13:16
Co-authored-by: joaquinelio <joaquinelio@gmail.com>
Co-authored-by: joaquinelio <joaquinelio@gmail.com>
Co-authored-by: joaquinelio <joaquinelio@gmail.com>
Co-authored-by: joaquinelio <joaquinelio@gmail.com>
Co-authored-by: joaquinelio <joaquinelio@gmail.com>
Co-authored-by: joaquinelio <joaquinelio@gmail.com>
Co-authored-by: joaquinelio <joaquinelio@gmail.com>
Co-authored-by: joaquinelio <joaquinelio@gmail.com>
Co-authored-by: joaquinelio <joaquinelio@gmail.com>
Co-authored-by: joaquinelio <joaquinelio@gmail.com>
@joaquinelio
Copy link
Member

@Derlys
Me llegó el mensaje "closed"... je quiero creer que fue un dedo mal puesto, ¿no?
o será que te arrepentiste...
falta un reviewer y va para el sitio

@Derlys
Copy link
Contributor Author

Derlys commented Sep 4, 2020 via email

@joaquinelio
Copy link
Member

Si está mal, se puede cerrar y hacer uno nuevo
o mejor corregirlo...

¿qué es lo que está mal?

@Derlys Derlys reopened this Sep 5, 2020
@Derlys
Copy link
Contributor Author

Derlys commented Sep 5, 2020

Lo que te quiero decir es que no se que se debe hacer ahora con esto, dice changes aprobado ahora que sigue?

@joaquinelio
Copy link
Member

Ah, nada de tu lado hasta que alguien más lo vea.

el +1 significa otro reviewer, una segunda revisión...
andamos escasos de reviewers asi que solo hay que esperar.

Copy link
Contributor

@vplentinax vplentinax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hay algunas palabras que pueden cambiarse para no romper el flujo de lectura pero se puede arreglar en alguna revisión posterior de archivos traducidos.

@joaquinelio joaquinelio merged commit fda8501 into javascript-tutorial:master Sep 8, 2020
@javascript-translate-bot

Thank you 💖 I updated the Progress Issue #17 🎉 🎉 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants