-
Notifications
You must be signed in to change notification settings - Fork 27
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
Add JDK 21 to build matrix #187
Conversation
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.
Changes look good. 40 CI builds is a quite numbers indeed. Part of me wants to say that all these builds are around 1min and the activity on this repository is low at the moment. But we might want to reduce the number of concurrent builds either by adding more excludes or by reengineering slightly the configuration files to have 2 separate steps or 2 workflows:
- one testing all supported Java versions across all supported operating sytems with a single Python version
- one testing the code generation across all Python versions (adding 3.12 to the mix) for a single Java version
...instead of trying to be clever with a concise include list.
1b411a0 cuts down the number of jobs so that all Java versions are tested with Python 3.8 and all Python versions are tested with Java 11. Python 3.12 builds fail though, as use of https://docs.python.org/3.8/library/imp.html needs to be replaced with https://docs.python.org/3.8/library/importlib.html. If that turns out to be an easy change, I'll fix that here, otherwise will remove 3.12 from the matrix for now. |
Largely based on https://discuss.python.org/t/how-do-i-migrate-from-imp/27885 and other discussions linked from python/cpython#104212.
7f99b6a fixes the builds and keeps Python 3.12, but definitely read that carefully along with the discussions linked from the commit message. I don't 100% know what I'm doing there. |
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.
Infrastructure changes all make sense and reduce the load of concurrent actions.
On the Python 3.12, the code changes look good. The output of mvn -f ome-xml/pom.xml clean generate-sources
under ome-xml/target/generated-sources
was generated using Python 3.8 up to 3.12 with this PR included. The content of the code-generated files is identical to the ones generated without this PR with Python 3.10.
Might need to exclude some Java 21 + Python combinations from the matrix.