-
Notifications
You must be signed in to change notification settings - Fork 115
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
Use JAXB from Maven #413
base: main
Are you sure you want to change the base?
Use JAXB from Maven #413
Conversation
Closes oracle-samples#412 This should work on all the JDKs.
0a76e38
to
21d6eec
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems reasonable to me. I believe I heard of this approach taken elsewhere too.
As I understand it the need for these dependencies comes from our (relatively old) version of ClojureScript. According to CLJS-2377 and bhauman/lein-figwheel#612 (comment) ClojureScript has now released a version that doesn't have this dependency anymore, so if Clara bumped its dependency to that ClojureScript version we wouldn't need to declare this dependency. However, we'd be blind to anything that broke Clara in older ClojureScript versions unless we did further testing. We could create an "old ClojureScript" build similar to the changes made in #358 to test against newer Clojure/ClojureScript, have that build add these dependencies, and then bump the version of ClojureScript in the primary build to a newer one. This would ensure that we maintained compatibility with older ClojureScript versions. The reason I see for doing it this way is that as is, we'd be blind to any actual runtime dependency in the JVM version on these additional dependencies since all tests would pass and users would then be unable to use the jar (unless they added the dependencies). I'm a bit hesitant to assume that no Clara JVM dependencies, now or in the future, won't have an actual dependency on these sorts of "systems level" dependencies being provided. That said, I don't see it as a huge risk and in any event it is something that would quickly become apparent upon attempted use of Clara, so if what I'm describing won't work/has downsides I'm missing I don't mind merging this too much. As an ancillary benefit, any performance testing we do against ClojureScript should really be against a newer version, even if we maintain compatibility against older versions. Thoughts? I'm particularly interested in thoughts from anyone who keeps up with the ClojureScript ecosystem more than I do. |
@WilliamParker It makes more sense to me for Clara to build against fairly modern versions of clj and cljs as the default and have a few "older" versions profiles to test out level of a minimum required version. Perhaps something like 1.9 of clj, with some later 1.9.x of cljs as the default, then a profile for more cutting edge clj 1.10 and cljs 1.10.x and a profile for a minimum tested version we consider Clara to claim to have compatibility with. We do have to keep in mind that cljs moves quite a bit more quickly than clj and in its fairly fast paced releases, there are sometimes things that break, but then are later resolved (fairly rare still). So If you say, min support for cljs v1.8.x, it isn't too feasible to build against all versions of cljs from that minimum upwards. Just have to test a few versions and interpolate that things seem to be fine, but haven't actually been ran at every release along the spectrum. |
I figure this was a good thing for me to resolve for the next release, but here is the issue: I can update Clojure and ClojureScript in the But then the PhantomJS tests fail, though, and it appears to be a problem with PhantomJS's ES6 support. The issue is here: ariya/phantomjs#13895 . Then I discovered (did not know) that PhantomJS is officially dead, and Everyone Should Use Headless Chrome. So this seems like a little bit of work, including remembering how to Travis CI again. |
Hi @eraserhd The CLJS implementation really needs an overhaul due to pitfalls in how it is done. I think What worries me some is that locally I've ran into a lot of CLJS sort of failures doing things like compiling and testing due to the issues w/how CLJS rules are compiled via a sneaky/not-really-supported Perhaps I can try to revisit this yet again, or try to just do a single issue to stop using phantom and see if it still works as-is for now... |
Closes #412
This should work on all the JDKs. I've tested it on 8, 10, and 11.