don't claim full JSR-223 compliance#4733
Conversation
|
review by @adriaanm |
|
LGTM! |
don't claim full JSR-223 compliance
|
Sorry, I've been busy babysitting grandkids this evening. But frankly, in the current state of the code, given the stated interface contract of ScriptEngine and its explicit injunction that all implementations must implement all methods, I think it's a mistake to claim any JSR-223 support at all. |
|
I haven't fully grokked the issue, but isn't that a bit harsh? This support was contributed by someone who's presumable using it, so it must work enough for some, I think. Could you suggest a compromise in wording between claiming no support at all and the current one? |
|
I'm sorry for the trouble the overenthusiastic wording caused you, and would be happy to rectify so that others can learn from your experience. |
|
"ScriptEngine is the fundamental interface whose methods must be fully functional in every implementation of this specification." may be harsh, but really doesn't seem to leave a lot of room for subsets. http://docs.oracle.com/javase/7/docs/api/javax/script/ScriptEngine.html. If Scala simply shrugs off bugs in this with "out of scope, nobody cares", then fine...but why even claim its there? |
|
I really just don't agree that Oracle's wording there requires us to ship either 100% support or nothing. As for "why even claim it's there", I think there are two very good and sufficient reasons for that: 1) the current level of support is already useful and much better than nothing, and 2) it's a base for further improvement, which we hope will be forthcoming from the community. It is not true that "nobody cares", nor did we say that, nor do we feel that way about it. It's just that we can't do everything ourselves. I stand by the Scala team's decisions here and by the revised wording in this PR. |
|
I agree with Seth. We certainly don't want to mislead anyone with our wording, and we're happy to accept further improvements to bring our implementation into full compliance. We're quite the opposite of indifferent to any improvement or fix for Scala, but we have to be clear about which areas we're able to spend our time in. Implementing JSR 223 is not one of those areas, but we'll do our very best to enable everyone to contribute to it! Thanks for sharing your concerns. |
|
Your call, of course. When I mentioned elsewhere that JSR-223 was broken in the last 2.11 point releases that I looked at around this issue, I was pointed to the 2011 comment "This is currently out of scope for the Scala team. We don't see much demand for JSR-223" which I paraphrased as "out of scope, nobody cares". There may very well be some use case somewhere for a scripting API whose hosting container can't read or write values to the script environment; JSR-223 expects it to work. Apparently there was some point at which that did work because an example is in the published 2.11 overview. As of the last time I tried something similar, it simply failed silently. My own suspicion is that it was later changes to the interpreter that are now causing the failures. |
|
I tried it just now and the example in the overview involving
|
|
(mentioning @rjolly in case he's still interested) |
|
The official slogan on the Scala buttons they distribute at conferences is, "Out of scope, out of mind." I think it's an OOP joke. I commented on one of the tickets that I'll resuscitate an old refactor that moved the ScriptEngine stuff out of IMain. That didn't include feature completeness, but I'll make an effort. The "fully functional and operational death star" language in the API is because of the optional interfaces. |
|
I will be happy to investigate these issues, but first @adriaanm could you review my PR #4456 so that it gets in eventually ? It is meant to exercise the "manifest class path" mechanism, which is quite complex, and I would feel much more confortable about JSR 223 support with it being finally merged. |
|
Great, thank you! I'll ping Jason, since he already looked at it (a while ago -- sorry!) |
in response to the discussion at SI-8422