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

Cambia copy para solo copiar archivos de idioma elegido #1425

Merged
merged 12 commits into from
Aug 7, 2023

Conversation

unjust
Copy link
Contributor

@unjust unjust commented Jun 29, 2023

Ahora el script solo analiza que el README de idioma (pt o es) esta copiado, pero si hay otros markdowns, copian todo.

Ahora el comportamiento esta bien con README
sin --locale o --locale=es : README.md -> README.md
--locale=pt : README.pt.md -> README.md

pero con otros archivos con idioma, no importa el local : otroArchivo.md y otroArchivo.pt.md estan copiada tal cual., todos Ejemplo en un proyecto actual

Seria bueno copiar solo los archivos de idioma, entonces con markdowns adicionales tambien solo copiamos
sin --locale o --locale=es : otroArchivo.md -> otroArchivo.md
--locale=pt : otooArchivo.pt.md -> otroArchivo.md

Casos para considerar:

  • markdowns que solo tiene una version en la idioma por defecto tal vez debe ser copiado/disponible no importa el local
  • links en markdowns de una idioma no deberia agregar la idioma a su extension (por ejemplo un link de README.md hacia otroArchivo.pt.md solo deberia decir otroArchivo.md

@mfdebian mfdebian self-requested a review July 7, 2023 15:54
@mfdebian mfdebian added enhancement New feature or request in progress Alguien ya está trabajando en esto (para no pisarnos) labels Jul 7, 2023
@mfdebian mfdebian added this to the v6.4 milestone Jul 7, 2023
Copy link
Collaborator

@mfdebian mfdebian left a comment

Choose a reason for hiding this comment

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

gracias @unjust 🥳

hay algo que falte en este PR? se invoca una función llamada getAllFilesInDir pero no está declarada en el script, será que se quedó afuera de este PR? 😁

@mfdebian mfdebian changed the base branch from main to next July 10, 2023 19:46
@unjust unjust modified the milestones: v6.4.0 , Next release Jul 10, 2023
@unjust unjust marked this pull request as draft July 14, 2023 14:45
@unjust unjust changed the title Primer intenta a cambiar copy para solo copiar archivos de idioma eligido Cambia copy para solo copiar archivos de idioma elegido Jul 14, 2023
@unjust unjust marked this pull request as ready for review July 14, 2023 23:04
Copy link
Collaborator

@mfdebian mfdebian left a comment

Choose a reason for hiding this comment

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

@unjust aún me toca seguir revisándolo bien, pero tengo que hacer almuerzo ahora y luego tenemos reuniones, pero quería mostrarte que me está arrojando error

Al ejecutar el comando de la siguiente manera:

$ npm run create-cohort-project projects/04-burger-queen-api ~/Documents/ DEV100

Donde no estoy pasando un atributo locale, y por lo tanto el comportamiento esperado es el de que utilice como locale la defaultLocale ('es'), sin embargo, debido a la condición descrita que mencioné dentro de este review, a la función getFilesWithLocales sólo le llega al array [ 'pt' ] como argumento, pero en cualquier caso, el error que me arroja es el siguiente:

[Error: ENOENT: no such file or directory, unlink '/home/alpi/Documents/DEV100-burger-queen-api//home/alpi/Documents/DEV100-burger-queen-api/README.pt.md'] {
  errno: -2,
  code: 'ENOENT',
  syscall: 'unlink',
  path: '/home/alpi/Documents/DEV100-burger-queen-api//home/alpi/Documents/DEV100-burger-queen-api/README.pt.md'
}

Que al parecer tiene relación con un join extra que se debe estar invocando por ahí en la función getFilesWithLocales, ahorita después de almuerzo y saliendo de las reus lo reviso con más detenimiento, que no he alcanzado a llegar a esa función, para arreglar ese comportamiento.

Por lo demás, muchas gracias por el orden y claridad del código, estoy de acuerdo con tener una defaultLocale y también con que comencemos a crear esa suite de tests para los scripts! 😊

scripts/create-cohort-project.mjs Show resolved Hide resolved
scripts/script-utils.mjs Outdated Show resolved Hide resolved
scripts/create-cohort-project.mjs Show resolved Hide resolved
package.json Show resolved Hide resolved
@unjust
Copy link
Contributor Author

unjust commented Jul 17, 2023

Quiero comentar que un comportamiento de este script (como antes) es que si hay un documento que solo tiene version en el local por defecto, va a quedar en el proyecto resultado - no importa si no esta en el locale elegido.
Osea, siempre va a tener un version de cada documento en el proyecto, pero si no hay un version en la idioma elegido, este documento va a estar en español.

README.md
README.pt.md
Guia.md

resulta en

README.md  (pt)
Guia.md    (es)

@unjust unjust removed the in progress Alguien ya está trabajando en esto (para no pisarnos) label Jul 18, 2023
@unjust unjust modified the milestones: 6.5x, next release Jul 21, 2023
Copy link
Collaborator

@mfdebian mfdebian left a comment

Choose a reason for hiding this comment

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

Quizás podemos seguir revisándolo con más detenimiento, pero creo que haciendo ese cambio se comporta como esperamos y los tests siguen pasando 😊

Muchas muchas gracias @unjust 🙏

scripts/create-cohort-project.mjs Outdated Show resolved Hide resolved
Ivy Feraco and others added 2 commits July 31, 2023 14:49
Co-authored-by: Alfredo González <12631491+mfdebian@users.noreply.github.com>
@unjust unjust force-pushed the md_intl_copy_1413 branch from 8137765 to cb1d9ff Compare July 31, 2023 19:50
Copy link
Collaborator

@mfdebian mfdebian left a comment

Choose a reason for hiding this comment

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

No he alcanzado a revisar todo el código 😬 se me fue la hora 😅 , quizás mañana juntas lo podemos ver con más detalles, pero probando su uso, ahora sí tiene el comportamiento esperado 😄

Mil gracias @unjust !!

@unjust unjust merged commit f0d2f0c into Laboratoria:next Aug 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Arregla el paso de copiar archivos cuando crea proyecto para solo copiar la idioma elegido
2 participants