tkaymak commented on code in PR #38453:
URL: https://github.com/apache/beam/pull/38453#discussion_r3225387922
##########
buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy:
##########
@@ -1516,72 +1516,66 @@ class BeamModulePlugin implements Plugin<Project> {
project.tasks.analyzeDependencies.enabled = false
}
- // errorprone requires java9+ compiler. It can be used with Java8 but
then sets a java9+ errorproneJavac.
- // However, the redirect ignores any task that forks and defines either
a javaHome or an executable,
- // see https://github.com/tbroyer/gradle-errorprone-plugin#jdk-8-support
- // which means errorprone cannot run when gradle runs on Java11+ but
serve `-testJavaVersion=8 -Pjava8Home` options
- if (!(project.findProperty('testJavaVersion') == '8')) {
- // Enable errorprone static analysis
- project.apply plugin: 'net.ltgt.errorprone'
+ // Enable errorprone static analysis
Review Comment:
Nit / scope: removing the dead Java-8 gate around the errorprone block is a
fine cleanup (Java 8 isn't in the supported `testJavaVersion` set
`['11','17','21','25']` anymore), but it's unrelated to the Spark 4 test wiring
this PR is otherwise about, and it accounts for most of the diff's visual size.
Happy to approve as-is — just a one-line mention in the description would help
future archaeology, or split it out into a tiny separate PR if you'd prefer. No
objection either way.
##########
.github/workflows/beam_PreCommit_Java_Spark_Versions.yml:
##########
@@ -87,12 +87,17 @@ jobs:
github_job: ${{ matrix.job_name }} (${{ matrix.job_phrase }})
- name: Setup environment
uses: ./.github/actions/setup-environment-action
+ with:
+ java-version: |
+ 17
+ 11
- name: run sparkVersionsTest script
uses: ./.github/actions/gradle-command-self-hosted-action
with:
- gradle-command: :runners:spark:3:sparkVersionsTest
+ gradle-command: :runners:spark:3:sparkVersionsTest
:runners:spark:4:build
Review Comment:
👍 Confirms the `requireJavaVersion=17` plumbing from #38324 plus the new
`forkJavaVersion` → `testJavaVersion` fallback in this PR will make
`:runners:spark:4:build` fork to JDK 17 for both compile and test on a
Java-11-default CI box. With `actions/setup-java`'s multi-line `java-version`
putting `JAVA_HOME` on the last-listed entry (11), Spark 3 keeps its primary
JDK and Spark 4 forks via `-Pjava17Home=$JAVA_HOME_17_X64`. Clean.
##########
.github/workflows/beam_PreCommit_Java_Spark_Versions.yml:
##########
@@ -15,21 +15,21 @@
# specific language governing permissions and limitations
# under the License.
-name: PreCommit Java Spark3 Versions
+name: PreCommit Java Spark Versions
on:
push:
tags: ['v*']
branches: ['master', 'release-*']
paths:
- 'runners/spark/**'
- - '.github/workflows/beam_PreCommit_Java_Spark3_Versions.yml'
+ - '.github/workflows/beam_PreCommit_Java_Spark_Versions.yml'
pull_request_target:
branches: ['master', 'release-*']
paths:
- 'runners/spark/**'
- 'release/trigger_all_tests.json'
- - '.github/trigger_files/beam_PreCommit_Java_Spark3_Versions.json'
+ - '.github/trigger_files/beam_PreCommit_Java_Spark_Versions.json'
Review Comment:
Two trigger-file follow-ups I'm happy to roll into the small cleanup PR I'm
already drafting for #38255:
1. The renamed workflow references
`.github/trigger_files/beam_PreCommit_Java_Spark_Versions.json` here, but no
file with that name exists — worth adding a stub for symmetry with the other
workflows that use this mechanism, otherwise the `pull_request_target.paths`
trigger is a no-op for that path.
2. The orphaned
`.github/trigger_files/beam_PreCommit_Java_Spark4_Versions.json` (left over
after #38255 deleted its workflow) is still on master and should be swept up
too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
