-
Notifications
You must be signed in to change notification settings - Fork 46
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
Update cookbook versions for 16.0.0 release #348
Comments
Let me know if I can help with anything here @amoeba |
Hey @raulcd, this slipped. I'll work on a PR for the Java refactor this week. |
I put a PR up at #350. I ran into one snag which may require rewriting all Java examples to have an explicit main class. I'd appreciate a review if you have a minute. |
amoeba
added a commit
that referenced
this issue
May 23, 2024
In #347 we found the way we have been running cookbooks for Java (JShell) doesn't work well with JPMS which was introduced in Arrow 16. This refactors `javadoctest.py` to run examples directly with Maven using `exec:java` instead of with JShell. This PR also bumps the Java source/target version to 11 to fix some compiler errors and fixes a few compilation errors in cookbook code. I ran into one snag will require a follow-up commit to this PR: The way the examples in [substrait.rst](https://raw.githubusercontent.com/apache/arrow-cookbook/main/java/source/substrait.rst) are written doesn't work with my approach. My approach splits each code snippet into its `import` statements and non-`import` statements, puts the imports outside the main class definition and puts the non-imports inside the class's main method. This works fine for every example except [substrait.rst](https://raw.githubusercontent.com/apache/arrow-cookbook/main/java/source/substrait.rst) which needs some of its code to be defined in the main class, e.g., We probably generally want to support examples that need this so I think we may need to rewrite all the Java cookbooks to have an explicit main class. @lidavidm suspected this might be the case in #347 (comment) but I do wonder if there is still a way to avoid this. Any ideas welcome. Fixes #347 Related #348
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
We can normally get away with just bumping versions in a minor PR but two issues have already come up in early testing for the 16.0.0 release so I thought I'd track those and anything else we find here ahead of the release.
The text was updated successfully, but these errors were encountered: