-
Notifications
You must be signed in to change notification settings - Fork 695
[GEODE-10465] Migrate Apache Geode to Java 17: JAXB Integration, Module System Compatibility, and Test Infrastructure Modernization #7930
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
Changes from 2 commits
07da4b8
ea64411
b645c9b
720895f
1878261
ac5f7a9
ab90720
179993d
1fbdad2
fe32358
da0855d
6a283ab
faba36d
89604c4
3dad40a
c91e2af
7dc4399
4be2035
58f0195
126854d
f96a745
49af81b
5fc7c38
172176c
e20ed7d
7362f67
5e87b47
43ccf7d
cda8f4f
f64b8a5
ac51a7b
691a119
f57db6b
f511984
f783780
f724ab9
49dd350
191a28c
ede537c
c3c9322
07c7825
1052c4f
1d4b939
3950d50
2626d55
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -148,9 +148,9 @@ public void compileOrderByClause(final int numOfChildren) { | |
| } | ||
|
|
||
| public void compileGroupByClause(final int numOfChildren) { | ||
| final List<CompiledPath> list = new ArrayList<>(); | ||
| final List<CompiledValue> list = new ArrayList<>(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just FYI with (3 different AI reviews and they all say the same)
public void compileGroupByClause(final int numOfChildren) {
final List<CompiledValue> list = new ArrayList<>();
for (int i = 0; i < numOfChildren; i++) {
list.add(TypeUtils.checkCast(pop(), CompiledValue.class));
}
// reverse to preserve original left-to-right order without O(n^2) insert-at-zero
java.util.Collections.reverse(list);
push(list);
}But this is just a 'group by' case so I don't think it matters since group by can't be big at all to matter
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Question: does the evaluator elsewhere require CompiledPaths?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @sboorlagadda , the evaluator components work entirely through the CompiledValue interface and don't need the specific CompiledPath type. |
||
| for (int i = 0; i < numOfChildren; i++) { | ||
| list.add(0, pop()); | ||
| list.add(0, TypeUtils.checkCast(pop(), CompiledValue.class)); | ||
| } | ||
| push(list); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,6 +40,10 @@ dependencies { | |
| implementation('com.fasterxml.jackson.core:jackson-databind') | ||
| implementation('io.swagger.core.v3:swagger-annotations') | ||
|
|
||
| // JAXB dependencies needed for Java 11+ | ||
| implementation('javax.xml.bind:jaxb-api') | ||
| implementation('com.sun.xml.bind:jaxb-impl') | ||
|
||
|
|
||
| // //Find bugs is used in multiple places in the code to suppress findbugs warnings | ||
| testImplementation('com.github.stephenc.findbugs:findbugs-annotations') | ||
| testImplementation('org.springframework:spring-test') | ||
|
|
||
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.
sourceCompatibility = 17 is repeated again inside whenReady. May be we can use toolchains; itβs the canonical way for JDK 17?
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.
Excellent observation @sboorlagadda . Let me use toolchains instead. Thank you