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

Fetch #435

Merged
merged 19 commits into from
Oct 30, 2020
Merged

Fetch #435

merged 19 commits into from
Oct 30, 2020

Conversation

carlosabud
Copy link
Contributor

No description provided.

@CLAassistant
Copy link

CLAassistant commented Oct 25, 2020

CLA assistant check
All committers have signed the CLA.

@joaquinelio
Copy link
Member

Hola
Todavia no lo revise pero te aviso que tiene ejercicios

uno ejercicio solo. en tu branch
https://github.com/carlosabud/es.javascript.info/tree/master/5-network/01-fetch/01-fetch-users
task.md y solution.md

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.

  • Varios acentos, si no usas las suggest acordate de ponerlos
  • Recordar que faltan task y solution
  • Me permití corregir los nros de linea
    necesitamos que coincidan con el inglés para poder hacer bien los pull
  • Cambie nombre para que Don Bot lo reconozca, agregué el pr que Don Bot no reconoció al issue17

5-network/01-fetch/article.md Outdated Show resolved Hide resolved
5-network/01-fetch/article.md Outdated Show resolved Hide resolved
5-network/01-fetch/article.md Outdated Show resolved Hide resolved
5-network/01-fetch/article.md Outdated Show resolved Hide resolved
5-network/01-fetch/article.md Outdated Show resolved Hide resolved
5-network/01-fetch/article.md Outdated Show resolved Hide resolved
5-network/01-fetch/article.md Outdated Show resolved Hide resolved
5-network/01-fetch/article.md Outdated Show resolved Hide resolved
5-network/01-fetch/article.md Outdated Show resolved Hide resolved
5-network/01-fetch/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 joaquinelio changed the title Traducción articulo Fetch Fetch Oct 25, 2020
@carlosabud
Copy link
Contributor Author

Buenas Joaquin, ahí realicé un par de actualizaciones tomando tus sugerencias.

Cualquier cosa avisame

@joaquinelio
Copy link
Member

Uh
Debí advertirte, no traducimos código, puede salir mal.
Y no lo podemos probar acá.
(Pero los devs somos perfectos y no cometemos errores, para qué probar)

Se traducen solamente // comentarios /* */
y usualmente mensajes entrecomillados (con cuidado los backticks) pero no los string que participan de codigo

salvo eso, el article creo que ya está, task y solution estan bien...
¿podrias poner codigo sin traducir?

@carlosabud
Copy link
Contributor Author

carlosabud commented Oct 26, 2020

@joaquinelio Creo que ahí estamos. Avisame!

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.

Qudó un js
5-network/01-fetch/01-fetch-users/_js.view/solution.js
lo más seguro es poner el original entero,

al otro js le copie la linea vieja,
5-network/01-fetch/01-fetch-users/_js.view/source.js
por lo menos el comentario quedo traducido... =)

Estaba tentado a dejarlo asi, probarlo localmente
el problema es que a veces hacen cambios de codigo y el pull es directo
es posible por ej que agreguen una linea usando la variable ingles y nadie se de cuenta de que el codigo se malogro

Mas par de cositas,

5-network/01-fetch/01-fetch-users/_js.view/source.js Outdated Show resolved Hide resolved
5-network/01-fetch/01-fetch-users/solution.md Outdated Show resolved Hide resolved
5-network/01-fetch/01-fetch-users/solution.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 👻

carlosabud and others added 4 commits October 26, 2020 22:55
Co-authored-by: joaquinelio <joaquinelio@gmail.com>
Co-authored-by: joaquinelio <joaquinelio@gmail.com>
@carlosabud
Copy link
Contributor Author

Listo. Perdón por las idas y vueltas.

/done

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.

👍

@carlosabud
Copy link
Contributor Author

@joaquinelio por las dudas, no me deja completar el PR. Supongo que hará falta que otra persona con permisos revise el PR y lo complete. Saludos!

@joaquinelio
Copy link
Member

Sí, solo maintainers pueden continuarlo.

  • El branch y el PR son tuyos, podes hacer y deshacer.
  • merge dentro del repo solo la puede hacer alguien con permisos,
  • la aprobación es cuestion de procedimiento, lo esperable es que dos personas aprueben y el bot de Ilya responde cambiando los cartelitos.
  • Vos podrias hacer review y aprobar el PR de otra persona, y aunque el BOT te ignore, el maintainter no
    (especialmente por la escasez de reviewers)

Ja, deberia poner esto en el curso de git filosofico
para complementar cualquier curso de comandos git

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.

Ok

@joaquinelio joaquinelio merged commit a766c14 into javascript-tutorial:master Oct 30, 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

5 participants