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

fix #1739 - Definición de una constante por fuera de un container me pide que inicie en minúscula #1754

Conversation

nicovio
Copy link
Contributor

@nicovio nicovio commented Aug 23, 2019

fix-1739-

@nicovio nicovio added this to the Wollok v.1.8.2 Hypatia milestone Aug 23, 2019
@nicovio nicovio requested a review from fdodino August 23, 2019 04:04
@nicovio nicovio self-assigned this Aug 23, 2019
@coveralls
Copy link

coveralls commented Aug 23, 2019

Coverage Status

Coverage remained the same at 89.409% when pulling 35dd73a on fix-#1739-definicion-de-una-constante-por-fuera-de-un-container-me-pide-que-inicie-en-minuscula into 449bb1c on dev.

Copy link
Collaborator

@fdodino fdodino left a comment

Choose a reason for hiding this comment

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

Más allá del cambio, no estoy realmente seguro de implementarlo, al menos no ahora que tenemos que liberar una versión. Me gustaría escuchar más opiniones y también falta al menos un test que pruebe que el validador no salta definiendo una constante por fuera de un WMethodContainer.

Cómo se comporta wollok-ts con las convenciones de nombres de variables en mayúscula/minúscula, @nscarcella ?

@nscarcella
Copy link
Member

@fdodino me agarrás con los pantalones bajos. Ahora mismo wollok-ts no se banca constantes a nivel package (no sabia que se podía).
Las otras validaciones de nombres en teoría se comporta como Wollok-xtend. Estoy justo pensando que no sé cómo definir sanities para esto porque no hay forma en wollok de ver si algo da warning... Podríamos pensar cómo hacer para tener eso sincronizado entre proyectos...
Ya creo el ticket para agregar constantes globales a wollok-ts (es un buen primer issue ;) ).

Si me apuran, yo no validaría reglas de nombres para las constantes. A veces quiero la CONSTANTE_GRANDOTA, pero si se paspan capaz validaría aceptando ambas formas.

@npasserini
Copy link
Member

npasserini commented Aug 23, 2019 via email

@nicovio nicovio requested a review from fdodino August 24, 2019 03:50
Copy link
Collaborator

@fdodino fdodino left a comment

Choose a reason for hiding this comment

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

Me faltaron los tests @nicovio (se ve que faltó un git add), pero bueno... sale a prod!

@fdodino fdodino merged commit 96bb765 into dev Aug 25, 2019
@fdodino fdodino deleted the fix-#1739-definicion-de-una-constante-por-fuera-de-un-container-me-pide-que-inicie-en-minuscula branch August 25, 2019 13:32
@nicovio
Copy link
Contributor Author

nicovio commented Aug 25, 2019

Me faltaron los tests @nicovio (se ve que faltó un git add), pero bueno... sale a prod!

Uh perdón, en realidad tengo esta duda: cómo (y donde) puedo testear que el validador no salta cuando definís una constante por fuera de un WMethodContainer?

@fdodino
Copy link
Collaborator

fdodino commented Aug 25, 2019

Ah, mala mía, no vi que era un XPECT test acá

		const LOTI = 2

Quizás faltaría agregar un const lotito = 3 para mostrar que también acepta ok minúsculas. Pero un detalle...

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.

5 participants