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

Adds parsing for objectives #86

Merged
merged 17 commits into from
Sep 27, 2024
Merged

Conversation

unjust
Copy link
Contributor

@unjust unjust commented Dec 11, 2023

addresses #85

Adds curriculum-parser learning-objectives command with learning-objectives.js which returns a format similar to what our curriculum expects: https://github.com/Laboratoria/curriculum/blob/main/scripts/build.mjs#L125-L149

for now curriculum is not changed and doesnt use the parser for this. There is an issue to add this and here is an idea of how it would look

learning objectives comes with two options:
-- validate - which validates if the objectives defined in data.yml are present in the translated files, this can catch errors of spellings, hierarchy and missing translations
-- validate --strict - with validate, strict will only show errors if a learning objective is undefined in all of the translations. this is helpful to eliminate noise if only some OAs are in one language. Example - ux objectives are normally only defined in spanish, so using strict will filter most of the ux, unless they are missing from every translation file

Here is a gist showing validate outputs and json output

@unjust unjust requested a review from mfdebian December 11, 2023 23:24
@unjust
Copy link
Contributor Author

unjust commented Dec 11, 2023

Thinking ... maybe we should just warn if an OA is not found in any translated file, since all the UX ones will always be missing from pt.

@unjust unjust self-assigned this Dec 12, 2023
@unjust unjust linked an issue Dec 12, 2023 that may be closed by this pull request
index.js Show resolved Hide resolved
Copy link
Contributor

@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.

¡Muchas gracias por el PR @unjust ! 😊

Creo que el cambio que pediría, a grandes rasgos es el de seguir la misma "API" que siguen los demás comandos, como el de project o topic, en el sentido de que ambas funciones, parseProject y parseTopic están siendo envueltas por la función createHandler, y es esa función la que invocará a las demás y evaluará la calidad de satisfecha o rechazada de las funciones e imprimirá su valor de retorno en caso de que haya result, o invocará la función printError en caso de que se devuelva un error. Entonces el comando de objectives debería retornar ese "error" y yo movería la lógica de los console.warning que quieres agregar a esa función printError, de esa manera se sigue el "flujo" que ocupan los demás comandos también (e imprime también la ruta de dónde se gatilla el error), y así también se puede generar en la función parseLearningObjectives un objeto de retorno donde se utilice la variable opts donde normalmente vendrían cosas como la versión, por ejemplo, ya que ahora la variable opts no se está utilizando en la función (y por eso explotan los checks actualmente). Dime qué opinas! si te hace sentido o no, creo que a grandes rasgos el feedback es sólo intentar que la API del comando objectives se parezca más a la de los demás comandos 😊 🤘

@unjust
Copy link
Contributor Author

unjust commented Jan 2, 2024

@mfdebian Por ahora la validacion es solo una herramienta que podemos usar corriendo curriculum parser de linea de comando.

Tengo que hacer otro cambio en curriculum repo (por ejemplo eso) si queremos que el build de curriculum toma esta output para generar sus jsons en dist.

Copy link
Contributor

@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.

Al ejecutar tanto el comando

curriculum-parser objectives ../curriculum/learning-objectives/ --strict false

Como

curriculum-parser objectives ../curriculum/learning-objectives/ --strict true

Recibí el mismo resultado, supongo que no era eso lo esperado; Lo recibido, creo, es lo mismo que hay en https://curriculum.laboratoria.la/data/learning-objectives.json, me imagino que no era el output esperado, pero quizás yo entendí mal, pero me parece que la intención del PR es recibir un warning (o error con strict = true?) de cuáles son los learning-objectives que no tienen un título en algún (o ambos) idioma(s), no sé si es esa la intención o no, me avisas si falta algo! 🙌

Por lo demás, te agradecería mucho si es que puedes agregarle tests a las funciones que agregarías 🙏 eso ayudaría mucho a testear fácil el output esperado para cualquier caso! 😊

README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Co-authored-by: Alfredo González <12631491+mfdebian@users.noreply.github.com>
@unjust
Copy link
Contributor Author

unjust commented Jan 8, 2024

Al ejecutar tanto el comando

curriculum-parser objectives ../curriculum/learning-objectives/ --strict false

Como

curriculum-parser objectives ../curriculum/learning-objectives/ --strict true

Recibí el mismo resultado, supongo que no era eso lo esperado; Lo recibido, creo, es lo mismo que hay en

Cambie el README y espero que es mas clara que --strict es solo un bandeja para usar con --validate si quiere strict ser "falso" solo no coloca el opcion.

curriculum-parser objectives ../curriculum/learning-objectives/ --strict --validate
curriculum-parser objectives ../curriculum/learning-objectives/ --validate

index.js Show resolved Hide resolved
Copy link
Contributor

@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.

Mil gracias @unjust por los cambios, creo que aún faltaría actualizar el README para que incluya correctamente los ejemplos, pero sólo para entenderlo bien bien, lo que propones tiene 3 líneas de ejecución por así decirlo, una que es de la forma:

curriculum-parser learning-objectives ../curriculum/learning-objectives/ y que retorna un JSON que incluye sus objetos tree, flat, table, etc... que podríamos utilizar en el repo de curriculum para generar el archivo learning-objectives.json no?

Luego está la opción con validate:

curriculum-parser learning-objectives ../curriculum/learning-objectives/ --validate

Que en mi caso retorna lo siguiente:

Learning objectives missing from intl:
js/data-types missing in langs: es,pt
js/testing missing in langs: es,pt
js/modules missing in langs: es,pt
js/async missing in langs: es,pt
scm/git missing in langs: es,pt
scm/github missing in langs: es,pt
front-end-web-dev missing in langs: es,pt
front-end-web-dev/components missing in langs: es,pt
front-end-web-dev/state missing in langs: es,pt
java/spring-framework/spring-web/request-param missing in langs: es
php/data-types missing in langs: es,pt
php/testing missing in langs: es,pt
php/package-management missing in langs: es,pt
wordpress/plugin-development missing in langs: es,pt
typescript missing in langs: es,pt
typescript/basic-types missing in langs: es,pt
agile-planning missing in langs: es,pt
agile-planning/scope missing in langs: pt
agile-planning/workplan missing in langs: pt
agile-planning/tasks missing in langs: pt
business-understanding missing in langs: es,pt
business-understanding/business-model missing in langs: pt
business-understanding/goals-and-kpis missing in langs: pt
business-understanding/benchmark missing in langs: pt
research/planning/research-plan missing in langs: pt
research/planning/desk-research missing in langs: pt
research/planning/research-sample missing in langs: pt
research/planning/participants missing in langs: pt
research/qualitative missing in langs: es,pt
research/qualitative/user-interviews missing in langs: pt
research/qualitative/exploratory-techniques missing in langs: pt
research/quantitative missing in langs: es,pt
research/quantitative/user-surveys missing in langs: pt
research/quantitative/analytical-techniques missing in langs: pt
research/analysis missing in langs: es,pt
research/analysis/process-information missing in langs: pt
research/analysis/experience-mapping missing in langs: pt
research/analysis/identify-patterns missing in langs: pt
research/analysis/synthesize-results missing in langs: pt
design-concept missing in langs: es,pt
design-concept/define-mvp missing in langs: pt
design-concept/design-references missing in langs: pt
design-concept/ideate missing in langs: pt
design-concept/prioritize-ideas missing in langs: pt
interaction-design missing in langs: es,pt
interaction-design/user-flows missing in langs: pt
interaction-design/usability missing in langs: pt
interaction-design/information-architecture missing in langs: pt
content-design missing in langs: es,pt
content-design/voice-tone missing in langs: pt
content-design/create-content missing in langs: pt
interface-design missing in langs: es,pt
interface-design/visual-design missing in langs: pt
interface-design/accesibility missing in langs: pt
interface-design/responsive-design missing in langs: pt
design-prototype missing in langs: es,pt
design-prototype/basic-prototypes missing in langs: pt
design-prototype/high-fidelity missing in langs: pt
design-prototype/advanced-interactions missing in langs: pt
usability-analysis missing in langs: es,pt
usability-analysis/moderated-testing missing in langs: pt
usability-analysis/heuristic-analysis missing in langs: pt
usability-analysis/non-moderated-testing missing in langs: pt
design-pitch missing in langs: es,pt
design-pitch/design-feedback missing in langs: pt
design-pitch/design-presentation missing in langs: pt
design-specs missing in langs: es,pt
design-specs/file-organization missing in langs: pt
design-specs/use-cases missing in langs: pt
design-specs/project-documentation missing in langs: pt
design-systems missing in langs: es,pt
design-systems/atomic-design missing in langs: pt
design-systems/design-system missing in langs: pt
==> 73 objectives missing from intl
----
└── ../curriculum/learning-objectives/

Y en ese caso no retorna un JSON.

y la 3era opción que sería con validate y strict:

curriculum-parser learning-objectives ../curriculum/learning-objectives/ --validate --strict

Que en mi caso retorna:

Learning objectives missing from intl:
js/data-types missing in langs: es,pt
js/testing missing in langs: es,pt
js/modules missing in langs: es,pt
js/async missing in langs: es,pt
scm/git missing in langs: es,pt
scm/github missing in langs: es,pt
front-end-web-dev missing in langs: es,pt
front-end-web-dev/components missing in langs: es,pt
front-end-web-dev/state missing in langs: es,pt
php/data-types missing in langs: es,pt
php/testing missing in langs: es,pt
php/package-management missing in langs: es,pt
wordpress/plugin-development missing in langs: es,pt
typescript missing in langs: es,pt
typescript/basic-types missing in langs: es,pt
agile-planning missing in langs: es,pt
business-understanding missing in langs: es,pt
research/qualitative missing in langs: es,pt
research/quantitative missing in langs: es,pt
research/analysis missing in langs: es,pt
design-concept missing in langs: es,pt
interaction-design missing in langs: es,pt
content-design missing in langs: es,pt
interface-design missing in langs: es,pt
design-prototype missing in langs: es,pt
usability-analysis missing in langs: es,pt
design-pitch missing in langs: es,pt
design-specs missing in langs: es,pt
design-systems missing in langs: es,pt
==> 29 objectives missing from intl
----
└── ../curriculum/learning-objectives/

Y tampoco retorna un JSON.

Creo que lo que no me queda claro aún (disculpa 🙈 ) es cuál es la diferencia entre --validate y --validate --strict, ambos parecieran haltear la ejecución del programa y no retornar ningún JSON al final, pero sobretodo, el README no explica aún el caso de uso que los diferencia, aún no entiendo bien la diferencia entre ambos 🙈 si quieres mañana lo conversamos un poco más y vemos cómo podemos agregar esa documentación al README

Por lo demás, muchas gracias por tomarte el tiempo @unjust y disculpa que me cueste tanto entenderlo bien 🤣

README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
unjust and others added 2 commits January 10, 2024 13:07
Co-authored-by: Alfredo González <12631491+mfdebian@users.noreply.github.com>
@unjust
Copy link
Contributor Author

unjust commented Jan 15, 2024

@mfdebian me avisas si has probado. Creo la duda que queda (aparte si el codigo es hecho bien) es si seria util incluir los opciones de repo (no creo) y version (quizas).

Copy link
Contributor

@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.

Hola @unjust! volví a ejecutar el comando en mi local, veo que la función para devolver ese JSON con las propiedades tree, flat, intl y table funciona bien, pero a mi aún no me queda claro el resto de la funcionalidad del programa, al ejecutar el comando:
curriculum-parser learning-objectives ../curriculum/learning-objectives/ --validate
El programa se bota y no sigue ejecutándose, incluso al revisar el exit status del programa, me sale con código 1, es decir, ocurrió un error en la ejecución del programa, ¿es ese el comportamiento esperado? ¿qué pasa en el caso de que no arroje errores? qué debería retornar? 😁

porfa ayúdame a entender bien esas últimas 2 preguntas y ya lo aprobamos nomas, el código me parece ok! 😊 🙌

README.md Outdated Show resolved Hide resolved
lib/learning-objectives.js Outdated Show resolved Hide resolved
@unjust
Copy link
Contributor Author

unjust commented Mar 21, 2024

@mfdebian muy agredecida de este feedback. No me di cuenta que aun habia errores cuando no deberia ser.
Vas a ver ahora que cuando corre el script sin --validate siempre debe tener un exit status 0
Y cuando corre con validate y no hay errores tambien debe tener un exit status 0

Con --validate si hay objetivos invalidos, van a tirar errores y salir con status 1

Agregue algunos tests de cubrir los casos.

README.md Show resolved Hide resolved
@unjust
Copy link
Contributor Author

unjust commented Apr 4, 2024

Compare el json actual de OAs con el output de parser y parece igual con este diff
Entonces si queremos usar el parser para generar los objetivos con el build script, creo podemos reemplazarlo normal

Copy link
Contributor

@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.

Unos últimos pequeños cambios para que quede okey y listo para mergear 🙈 gracias @unjust !!! 🙌

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
lib/learning-objectives.js Outdated Show resolved Hide resolved
lib/learning-objectives.js Outdated Show resolved Hide resolved
lib/learning-objectives.js Outdated Show resolved Hide resolved
lib/learning-objectives.js Outdated Show resolved Hide resolved
unjust and others added 2 commits April 8, 2024 11:11
Co-authored-by: Alfredo González <12631491+mfdebian@users.noreply.github.com>
Copy link
Contributor

@fakel fakel left a comment

Choose a reason for hiding this comment

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

Todo parece estar ok, traté de ponerme al día con la conversación y parece que no hay problemas, si necesitas apoyo con el release me comentas, veo que no es algo que rompa la compatibilidad previa asi que no debería haber inconvenientes

@unjust unjust merged commit bc9d44c into Laboratoria:main Sep 27, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Verificamos los OAs yaml en el parte de curriculum parser
3 participants