-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
[BEAM-2530] More fixes/upgrades for Java 9 compatibility #4685
Conversation
8c20983
to
000fc7a
Compare
Ok, last fixes applied, a really neat progress, notice that almost all Java 9 specific parts are hidden in self-activated profiles, its functionality should be moved into gradle when the time is appropriate. I will soon start to fill specific JIRAs for the pending breaking tests and once that the fix for classloading gets decided and merged (#4497 or #4514). |
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.
Nice. LGTM.
sdks/java/io/xml/pom.xml
Outdated
</activation> | ||
<dependencies> | ||
<!-- | ||
These deps should be explilctly defined for Java 9 compat |
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.
typo
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.
thx
pom.xml
Outdated
@@ -413,6 +417,61 @@ | |||
</build> | |||
</profile> | |||
|
|||
<profile> | |||
<id>java-next</id> |
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.
Cool name, but is there a reason for java-next
instead of java-9
? I actually guess that with the timelines we might be testing with Java 8, 9, 10 at the same time.
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.
Well the thing I was thinking is that we probably should define our target if we will go for the whole spectrum (9.10,11) or if we asume like the milestone will be the LTS version (11). I will rename it to 9 and we will see how we evolve it depending pn the future releases.
000fc7a
to
38ffebb
Compare
retest this please |
Apart of the fixes it introduces an auto activated profile when using Java 9.
Follow this checklist to help us incorporate your contribution quickly and easily:
[BEAM-XXX] Fixes bug in ApproximateQuantiles
, where you replaceBEAM-XXX
with the appropriate JIRA issue.mvn clean verify
to make sure basic checks pass. A more thorough check will be performed on your pull request automatically.