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

Proxy and Reflect #451

Merged
merged 12 commits into from Jan 25, 2021
Merged

Conversation

joaquinelio
Copy link
Member

@joaquinelio joaquinelio commented Nov 27, 2020

Finally!
1033 lineas
ok, la mitad vacias, la mitad de mitad codigo
pero es el mas largo y complejo del repo

  1. use trampa para trap, algo incomodo, pero es la trampa de osos que atrapa operaciones, ¿que otra cosa podia poner?
    podia dejar TRAP, todo el mundo la entiende

  2. El articulo original es excelente, pero deberia mejorar la granatuca y hacerla mas suave
    Fui muy liberal en la traduccion no técnica
    tambien extendi algunas cosas

  3. Linea 966, le corte un pedazo porque estaba mal.
    se podia arreglar dejandola y agregando algo como "no en el caso de ejemplo porque...",
    o agragando una linea al codigo de ejemplo.
    cuando lo resuelva Ilya, lo arreglamos aca, sera un sync como tantos

@javascript-translate-bot

Error: the article has another translator @maksumi in the Progress Issue #17.

@joaquinelio
Copy link
Member Author

joaquinelio commented Nov 27, 2020

I know, Mr Bot.
But I dont care.
I stoled it.

wtf I've stolen it

@vplentinax
Copy link
Contributor

Me lo pienso para comenzar esta revisión... Pronto lo haré

@joaquinelio
Copy link
Member Author

Me lo pienso para comenzar esta revisión... Pronto lo haré

podes hacerla en comodas cuotas...

pero si podes dar algo de tiempo, la sync es siempre prioridad
Y las mantengo pequeñas para minimizar conflicto, no deberia llevar mucho tiempo.

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.

Revision de tareas y del articulo hasta la línea 200.

1-js/99-js-misc/01-proxy/03-observable/solution.md Outdated Show resolved Hide resolved
1-js/99-js-misc/01-proxy/03-observable/solution.md Outdated Show resolved Hide resolved
1-js/99-js-misc/01-proxy/article.md Outdated Show resolved Hide resolved
1-js/99-js-misc/01-proxy/article.md Outdated Show resolved Hide resolved
1-js/99-js-misc/01-proxy/article.md Outdated Show resolved Hide resolved
1-js/99-js-misc/01-proxy/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 👻

Co-authored-by: ᐯᑭᒪEᑎTIᑎᗩ᙭ ᐯᑭ <34555644+vplentinax@users.noreply.github.com>
@joaquinelio
Copy link
Member Author

joaquinelio commented Dec 19, 2020

@vplentinax

No comprendo muy bien la frase, ¿podría mejorarse?

` 181
// casi original
El proxy debe reemplazar totalmente el objeto target en todo lugar. Nadie debe jamás hacer referencia al objeto target después de haberlo hecho el proxy. De otro modo sería fácil manipularlo.

    //¿puedo apartarme del original y repasar lo que es la envoltura (wrapper) ? 

// 1ra verion sobreexplicada
El proxy debe reemplazar completamente al objeto "target" que envolvió . El proxy debe supervisar o reemplazar todas sus operaciones: nadie debe jamás hacer referencia al objeto target saltando tal envoltura. De otro modo sería fácil desbaratarlo.

// creo que quedo bien simplificado y contundente
El proxy debe reemplazar completamente al objeto "target" que envolvió: nadie debe jamás hacer referencia al objeto target saltando tal envoltura. De otro modo sería fácil desbaratarlo.

`
¿ s entiende ?
tu dirás...

TLDR
si otro programador usa "target" sin el proxy, el estado del proxy puede romperse, a eso se refiere con
klo que aca me comí " y en todo lugar", ligado a "nadie debe acceder", ¿puedo considerar que se sobrentiende?
me parecio mas claro sin aclarar...

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.

Revisón hasta la línea 592. Ya vamos por la mitad. Realmente está muy bien, pocas cosas que arreglar.

1-js/99-js-misc/01-proxy/article.md Outdated Show resolved Hide resolved
1-js/99-js-misc/01-proxy/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 👻

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.

Me gustó, me parece más claro. Me refiero a lo que arreglaste en reformulacion 181 "proxy vs target" @joaquinelio

joaquinelio and others added 2 commits December 23, 2020 20:48
Co-authored-by: ᐯᑭᒪEᑎTIᑎᗩ᙭ ᐯᑭ <34555644+vplentinax@users.noreply.github.com>
@joaquinelio
Copy link
Member Author

@vplentinax
Reescribí linea 328 que seguia confusa.

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.

Pequeñas cositas, una vez aceptadas puedes hacer merge!

1-js/99-js-misc/01-proxy/article.md Outdated Show resolved Hide resolved
1-js/99-js-misc/01-proxy/article.md Outdated Show resolved Hide resolved

For example, `getName()` method accesses the private `#name` property and breaks after proxying:
Por ejemplo, el método `getName()` accede a la propiedad privada `#name` y falla por hacerlo proxy:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Por ejemplo, el método `getName()` accede a la propiedad privada `#name` y falla por hacerlo proxy:
Por ejemplo, el método `getName()` accede a la propiedad privada `#name` y se interrumpe después del proxy:

Copy link
Member Author

Choose a reason for hiding this comment

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

No, no se interrumpe, falla, porque no encuentra la variable.

Y no "después del proxy" en todo caso sería "después de convertirlo a proxy"
pero puse "POR" porque la falla es consecuencia de convertirlo.
Falla, por haberlo convertido en proxy.

No enconctré forma comoda de traducir el ubicuo "proxing" que traduje como "hacerlo proxy"
¡ proxificar ?

Copy link
Member Author

Choose a reason for hiding this comment

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

un problema que no vi de poner "hacerlo proxy" es q parece que convirtiera al metodo,
deberia aclarar que convierto el objeto entero o escribirlo mas vagamente

"...falla despues de hacer el proxy"
"...falla despues de la conversión a proxy"
"...falla depués de la proxificación. "

no me parece nada mal la idea. Es muy comodo tener la palabra proxificar.

tamb puedo sobreaclarar
"...falla depués de la proxificación del objeto user. "

@vplentinax , pienso poner
"...falla depués de la proxificación. "
¿que tal?

1-js/99-js-misc/01-proxy/article.md Outdated Show resolved Hide resolved
joaquinelio and others added 5 commits January 17, 2021 16:59
Co-authored-by: Valentiina VP <34555644+vplentinax@users.noreply.github.com>
Co-authored-by: Valentiina VP <34555644+vplentinax@users.noreply.github.com>
Co-authored-by: Valentiina VP <34555644+vplentinax@users.noreply.github.com>
@joaquinelio
Copy link
Member Author

Novedades

-Acepté (alguno de mala gana :P ) tus cambios.
-Menos uno que rechazo porque le erra (o yerra) al concepto, pero deje comentada una versión alternativa que creo mejor.
-También hubo 3 conflictos que acabo de resolver y podrias revisar. La mejora de Ilya es buena, mejor la explicacion y alternativas de manejo del metodo "revoke". lineas 966-972 (o podes cliquear los commits para ver las idas y venidas...)

@joaquinelio
Copy link
Member Author

Acepte todas las sug excepto linea 845 porque era un error conceptual que no puedo aceptar.

Es el unico articulo que falta de la seccion Js,
es el más largo de todo el repo,
el PR ya cumplio dos meses,

y la demora genera conflictos que tengo que andar resolviendo

El art es bueno, vale la pena el merge

@joaquinelio joaquinelio merged commit edf7335 into javascript-tutorial:master Jan 25, 2021
@javascript-translate-bot

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

@joaquinelio joaquinelio deleted the leproso branch January 25, 2021 02:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants