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 #1459 stack overflow ts wollok game #1460

Merged
merged 2 commits into from
Aug 13, 2018

Conversation

npasserini
Copy link
Member

No description provided.

@ghost ghost assigned npasserini Aug 13, 2018
@ghost ghost added the in progress label Aug 13, 2018
@coveralls
Copy link

Coverage Status

Coverage remained the same at 91.949% when pulling 6046232 on Fix-#1459-stack-overflow-ts-wollok-game into 1b44b1f on dev.

// TODO: Reportar un error del type system que sea más piola que Error in EValidator
log.fatal("Type inference failed", e)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

🥇 Otra cosa que quiero agregar es un log.warning cuando corra el builder, para estar seguros que corrió el TS cuando corra el producto... que lo deje en el error log

Copy link
Member Author

Choose a reason for hiding this comment

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

Sí. Un detalle ahí es que atrapa Exception y no Error. Es decir que si hubiera un Stack overflow seguiría pasando loq ue pasaba antes. Dudé de atrapar también error pero me pareció demasiado heavy. De hecho la política de Eclipse es que ante un stack overflow te propone reiniciar el Eclipse, desconfía de que las cosas puedan funcionar después de eso. Y lo mismo podría pasar con otros errores. Por eso no sé si es sano atrapar Error / Throwable.

No sé, por ahora le metí eso, es para pensar.

@npasserini npasserini merged commit 7d4d764 into dev Aug 13, 2018
@ghost ghost removed the in progress label Aug 13, 2018
@npasserini npasserini deleted the Fix-#1459-stack-overflow-ts-wollok-game branch August 28, 2018 02:59
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.

3 participants