-
Notifications
You must be signed in to change notification settings - Fork 479
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 Python 3.7, 3.8 and openjdk>8 to build matrix #372
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
@googlebot I signed it! |
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
.travis.yml
Outdated
# Python implementation. Lives in python/, tested with bazel. | ||
- language: python | ||
python: 3.7 | ||
dist: bionic |
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.
What's prompting this change? We currently have the dist set to trusty globally - do Python 3.7 and 3.8 need to specifically run with bionic?
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.
Yes, trusty does not support python 3.7 or 3.8. The Travis build for the first commit failed on Python 3.7 and on Python 3.8 with the reason: Unable to download 3.(7|8) archive. The archive may not exist. Please consider a different version.
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.
Got it, thanks for the clarification. Would you mind setting the global dist to bionic and seeing if everything still passes? I'd prefer to keep everything in sync if possible.
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.
Will do. 👍
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.
It appears that Java 8 is no longer available on bionic, as it is EOL. Can bump the dist down to xenial, or set it specifically for java8 and add later jdks to be sure they work as well.
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.
I like your second approach - let's default to bionic and set it to older dist versions as needed. Thanks for doing this!
ec09163
to
2a0693a
Compare
It appears that the java build has problems with newer jdks, specifically with javadoc in jdk11+, and oraclejdk10 quit with a deprecated message, so I've removed all non-LTS Oracle JDKs from the test matrix. This still leaves the failures for jdk11+, so shall I comment those out, or allow them to fail? |
Feel free to comment them out for now. I'll merge in the PR once done. |
rubocop still failing as it has from the start, though everything else is green. |
Per #369 (comment) , Python 3.6 is already tested, however #369 states:
So, adding python 3.7 and 3.8, which have some potentially breaking changes, to the build matrix for continuous testing.