-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Build google-cloud-functions-http module with Java 8. #9912
Conversation
Please, keep these integration tests excluded from the CI Jobs as there is no point to test on Java 8 something that only runs on 11. Notice also that Java 8 is the default source/target for integration tests, so removing the properties is enought. |
Im on the fence about the exclusion. On the one hand, yes those tests arent doing much of anything on 8. However that also means theres little reason to bother complicating the CI config to exclude the module. I think having the module excluded in CI largely just makes it more likely for the Java 8 build to be broken again without people noticing (much as it was to cause all this discussion), especially if you do plan to add automated tests that then require 11 to run and dont prevent them running on 8. Agreed on the properties, I just pushed what I had played with as experiment altering the earlier profile fix branch that I decided not to raise a PR for. Should have thought further on that bit. |
b4d2bf5
to
b353102
Compare
I pushed a new version that jsut removes the compiler properties as thats the main issue and can be updated by itself. I still think the CI exclusions should probably be removed too though. |
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.
LGTM.
If someone else think it's a good idea to remove the CI exclusion, I'm OK with it.
I'll merge this since we have various CI failures caused by the Java 11 requirement |
Alternative to #9908 (which does what I originally proposed doing / had also done but not raised yet) for discussion.
Fix for #9904.